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

MediaUploadWorker: Filter on PhotoResponse. #2172

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

scolsen
Copy link
Contributor

@scolsen scolsen commented Jan 10, 2024

  • Make path in PhotoResponse public so that we can use it directly when uploading media.
  • Filter submission deltas by response value instead of task type.
  • Minor stylistic tweaks in trailing lambdas.

@gino-m PTAL

Note that at this point we already show media upload statuses in the sync history fragment (though it may happen fast enough that one really only sees the "COMPLETED" state reported). If we want to report on the status of photos separately from their associated mutations, there's quite a bit more work to do.

- Make `path` in PhotoResponse public so that we can use it directly when uploading media.
- Filter submission deltas by response value instead of task type.
- Minor stylistic tweaks in trailing lambdas.
@scolsen scolsen requested a review from gino-m January 10, 2024 14:32
Copy link
Collaborator

@gino-m gino-m left a comment

Choose a reason for hiding this comment

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

Nice cleanup and steps towards media sync status!

@gino-m
Copy link
Collaborator

gino-m commented Feb 1, 2024

@scolsen Gentle ping, this is approved but some checks are failing.

@gino-m
Copy link
Collaborator

gino-m commented Mar 16, 2024

@scolsen Gentle ping. Could you please fix checks so we can merge this?

Also, is this a pure code health fix, or does it change user-visible behavior? When you can please link to an open issue for posterity.

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.

None yet

3 participants