Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix error when trying to render component for media without meta #16112

Merged
merged 1 commit into from
May 5, 2021
Merged

Fix error when trying to render component for media without meta #16112

merged 1 commit into from
May 5, 2021

Conversation

Gargron
Copy link
Member

@Gargron Gargron commented Apr 26, 2021

No description provided.

@Gargron Gargron added the bug Something isn't working label Apr 26, 2021
@ClearlyClaire
Copy link
Contributor

Not completely opposed to that refactoring (though I kinda agree with the DeepSource warning here), but could the actual fix be its own PR?
Also I don't understand how the meta attribute is set or what could cause it not to be set.

@Gargron
Copy link
Member Author

Gargron commented Apr 27, 2021 •
edited

Also I don't understand how the meta attribute is set or what could cause it not to be set.

It's set in the MediaAttachment callbacks (before_post_process :set_meta, actually, even before that, paperclip-av-transcoder sets that attribute, which is why we filter keys we want to keep when overwriting it) and it's never not set. But there exist old records that predate the meta attribute, since it is nullable and has no default.

ClearlyClaire
ClearlyClaire approved these changes Apr 29, 2021
View reviewed changes
Copy link
Contributor

@ClearlyClaire ClearlyClaire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not too sure about how the autoplay stuff is handled, but otherwise LGTM

@Gargron Gargron force-pushed the fix-render-error-on-no-file-meta branch from 069e75c to 95041df Compare May 5, 2021 17:54
@Gargron Gargron merged commit 351c744 into main May 5, 2021
@Gargron Gargron deleted the fix-render-error-on-no-file-meta branch May 5, 2021 19:16
chrisguida pushed a commit to Start9Labs/mastodon that referenced this pull request Feb 26, 2022
@Gargron @chrisguida
Fix error when trying to render component for media without meta (mas…
97dbe69 
…todon#16112)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ClearlyClaire ClearlyClaire ClearlyClaire approved these changes

Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@Gargron @ClearlyClaire