Migrate the OG scrape response to the generated GetOGResponse model - #6657
Draft
gpunto wants to merge 1 commit into
Draft
Migrate the OG scrape response to the generated GetOGResponse model#6657gpunto wants to merge 1 commit into
gpunto wants to merge 1 commit into
Conversation
Contributor
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
Contributor
SDK Size Comparison 📏
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Goal
Migrate the OG scrape response to the generated
GetOGResponsemodel, soenrichUrlno longer parsesinto the hand-written
AttachmentDto.Part of AND-1291
Implementation
OpenGraphApi.get()returns the generatedGetOGResponse, mapped to the domainAttachment.GetOGResponseAdapterso root-level custom fields are still collected intoextraData, which iswhat
AttachmentDtoAdapterdid for this endpoint.GetOGResponse,Images,ImageData,ActionandField. The last four come alongbecause Go's
GetOGResponseembeds the wholeAttachmentpayload, so the schema inherits every fieldan attachment can carry, not only the ones a scrape produces.
AttachmentDtostays: message and draft attachments still use it.Notes
GetOGResponseembeds the whole attachment shape, so it declares more than a scrape produces. The mappercovers all of it except
giphy, which is not mapped here: it has a single producer, the giphy slashcommand writing to a message attachment, so it cannot reach
/og. The slice that adopts the sharedAttachmentmodel owns it, and has to re-emit it intoextraData["giphy"]orAttachment.giphyInfo()stops finding gif urls;
closure.pyprints that reminder for any closure containingImages.file_size,image,mime_typeandnameare absent from the payload struct, so when they are sent theyarrive inside
custom. The mapper reads them back out and removes them, so they do not also sit inextraDataunder their wire names, which is how the hand-written DTO behaved.E2E
/ogis routed in the mock server, and it answers with a recorded message attachment that carries noduration, the one field the generated model requires non-null.HyperLinksTestscovers the unsplash andyoutube previews, so this would go red without
mock-server #64, which sends
durationon that response. Needs an E2E run withmock_server_branch:fix/og-durationbefore merging.Testing
GetOGResponseParsingTestcovers the collected custom data, an empty scrape falling back to domaindefaults, the undeclared root fields being mapped and removed from
extraData, andauthor_name/author_link, which are same-named on both sides so a swap would otherwise go unnoticed.enrichUrlagainst three pages (an article, an image page and a video page), logging theraw response body next to every field of the resulting
Attachment. The wire returnedtype,title,title_link,text,image_url,thumb_url,asset_url,og_scrape_urlandduration, all mappingcorrectly (
durationis response metadata with no domain field), andextraDatastayed empty, sonothing arrived under an unexpected name. The remaining declared fields were not seen on those three
pages, which is why they are mapped rather than dropped: three samples cannot show a field is never
sent, and losing one would be silent.