Stopping posts being made while rich-embed preview processing #2418
Summary
Closes #2418
Original change on prod is that if you hit enter too quickly after posting a link, it will not post with the rich embed preview. That will upload in a separate comment below after the comment is posted.
Steps to test
- Log in and go to a post
- Copy a link e.g.
https://www.google.com/ - Paste it into a comment on the post and repeatedly and rapidly hit enter until the preview is done
- it should post with the preview, waiting till the preview is ready.
- Repeat with an invalid URL e.g.
https://www.ks3fnj3n9.com - it should only post when it's been processed for an available preview - you should see the loading indicator that indicates when this is happening.
Estimated Regression Scope
This feature touches a few areas
-
It affects when a user can post by directly stopping the user from posting until progress equal to 100, when a rich-embed is detected. In the worst-case, this could conceivably stop users posting in some edge-case situation.
-
It uses the same progress mechanism as image uploads - I felt it made the most sense as there shouldn't be an overlap of uploading an image and a rich-embed. Regardless, tests one after the other are good.
added scoped labels
![Ben Hayward](data:image/svg+xml,<svg xmlns="http://www.w3.org/2000/svg" width="200" height="200"><rect fill-opacity="0"/></svg>)
Resolved by Brian Hatchetapproved this merge request
![Xander Miller](data:image/svg+xml,<svg xmlns="http://www.w3.org/2000/svg" width="80" height="80"><rect fill-opacity="0"/></svg>)
DeveloperSeems to wait to post after load whether it's a valid or invalid link. I don't see the difference in behavior though. Thumbnails are loading on sandbox, but I can see the delay when compared to production.
resolved all threads
- Developer
The difference in behavior is that on production, if you type in
hello here is a link https://www.google.comand hit enter instantly, the comment text will post, but the preview will appear on a new comment below when it's done.Unfortunately on sandbox there is still no API key for iframely . Moving to blocked.
https://fix-admin-firehose-1658.minds.io/api/v1/newsfeed/preview?url=https%3A%2F%2Fwww.google.com
{"status":403,"error":"Valid api_key is required to access API","meta":{"description":""}} added scoped label and automatically removed label
- Resolved by Ben Hayward
resolved all threads
- Developer
Note there's a lot of null errors being thrown on pro pages on sandboxes. I've had a look but can't find any information to help find the source, and as it's cross sandbox, it's not caused by this MR.
assigned to @edgebal
approved this merge request
- Developer
Looking good!
added scoped label and automatically removed label
assigned to @benhayward.ben and unassigned @edgebal
- Resolved by Ben Hayward
added scoped label and automatically removed label
- Developer
This change seems to introduce breaking behavior for invalid links. On production, when I specify https://www.ks3fnj3n9.com - a spinner loads and then disappears, which seems like proper behavior.
On the review site, the spinner loads but it is then replaced by a big, broken image.
- Developer
I don't see this behavior with these recent changes. Can you confirm?
- Developer
:looking:
- Resolved by Brian Hatchet
added scoped label and automatically removed label
added scoped label and automatically removed label
added scoped label and automatically removed label
- Developer
Sorry about that, it needed the iframely key
59 63 this.minds = window.Minds; 60 64 } 61 65 66 ngOnDestroy() { 67 this.attachmentSubscription.unsubscribe(); - Owner
potential unsubscribe from non initialised subscription
189 */ 190 async getPostPreview(message: string) { 191 if ( 192 !message || 193 this.attachment.getMeta().is_rich || 194 !this.attachment.containsRichEmbed(message) 195 ) { 179 196 return; 180 197 } 181 198 182 this.attachment.preview(message, this.detectChanges.bind(this)); 199 this.canPost = false; 200 201 try { 202 this.attachment.preview(message, this.detectChanges.bind(this)); // generate preview. 203 this.attachmentSubscription = this.attachment.attachmentProgress$ - Owner
Why are we making a subscription inside this function? What if a subscription already exists. I feel like I've commented this a few times already. Please research how Observables work.
added scoped label and automatically removed label
- Developer
I think the fix we're attempting is incorrect too. If someone wants to just hit enter before a preview they should. The fix should be to not show rich embed when there is message there
![:speech_balloon:](data:image/svg+xml,<svg xmlns="http://www.w3.org/2000/svg" width="64" height="64"><rect fill-opacity="0"/></svg> ":speech_balloon:"){kind=small}
