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.

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

Fix creation of covers for ActivityPub imports #3039

Merged
merged 1 commit into from
Nov 2, 2023

Conversation

dato
Copy link
Contributor

@dato dato commented Oct 16, 2023


Imports for other BookWyrm instances were showing without covers for me, and I think this is the cause?

Code seemed correct, as in, I thought value here would be a document. But then the comment refers to receiving JSON, in which case the code is not correct. For this reason I've left both.

`cover` comes as a JSON dict, but the code was looking for URL as
an attribute.

(This commit leaves the attribute access in place, just in case
`cover` is updated to serialize as Document proper.)
@prolibre
Copy link

Oh great! Thank you so much !

@hughrun
Copy link
Contributor

hughrun commented Oct 31, 2023

I can check this tomorrow.

The comment is confusing, because it refers to a "JSON blob" which initially made me think it meant a Blob.

value here is only the image field listed within the ActivityPub JSON for a given activity. It's called from set_field_from_activity.

I think what the comment is meant to convey is that sometimes value is just the url as a string (if it's pulled from an "attached" image), and sometimes it's originally a JavaScript object (if the image is in intrinsic part of the ActivityPub object, as in the case of a user or Edition).

@mouse-reeve
Copy link
Member

This seems like it works to me -- is there any reason it's still a draft?

I believe I meant that the value can either be a dict or a string, but I wrote it to support objects, possibly because the activitypub could be passed in still as an ActivityPub object, even though evidently it's coming in as a dict

@mouse-reeve mouse-reeve marked this pull request as ready for review November 2, 2023 21:44
@mouse-reeve mouse-reeve merged commit ae51dce into bookwyrm-social:main Nov 2, 2023
10 checks passed
@dato dato deleted the ap_image_url branch November 3, 2023 09:12
dato added a commit to dato/bookwyrm that referenced this pull request Jan 25, 2024
Conflicts:
	requirements.txt (changes ignored)
dato added a commit to dato/bookwyrm that referenced this pull request Jan 25, 2024
Conflicts:
	requirements.txt (Pillow version bump ignored)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants