-
Notifications
You must be signed in to change notification settings - Fork 4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add auto-generated social images for users #20193
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention:
... and 8 files with indirect coverage changes 📢 Thoughts on this report? Let us know!. |
Uffizzi Ephemeral Environment
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of comments I'm uncertain about. It also seems the net-new lines that aren't currently covered by tests are pretty important ones; how difficult would tests for those cases be to add here?
it "displays social image if present" do | ||
user.profile.update(social_image: "https://socialimage.com/image.png") | ||
get "/#{user.username}" | ||
# Does not include the word, but does include the SVG |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Misleading comment: This isn't an SVG. I'm also not sure what word isn't being included...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@klardotsh sorry I think these are copy/pasted comments 🙃
|
||
it "displays fallback image if social image not set" do | ||
get "/#{user.username}" | ||
# Does not include the word, but does include the SVG |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What word isn't being included?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GitHub bot on Slack still thinks this is pending my review - my remarks about needing to update comments still stand.
What type of PR is this? (check all applicable)
Description
This adds a generated social image for users, re-using some functionality from what we made for articles.
Related Tickets & Documents
QA Instructions, Screenshots, Recordings
Check if Uffizzi gets this right
Added/updated tests?
We encourage you to keep the code coverage percentage at 80% and above.
have not been included