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

Deactivated Account Changes #2146

Merged
merged 12 commits into from Dec 21, 2021
Merged

Deactivated Account Changes #2146

merged 12 commits into from Dec 21, 2021

Conversation

rickyrombo
Copy link
Contributor

@rickyrombo rickyrombo commented Dec 13, 2021 •
edited

Description

  • AUD-1119 Update discovery API to not include download or segments on deleted/owner-deactivated tracks, and to not stream deleted/owner-deactivated tracks
  • AUD-1118 Updates discovery challenge manager to exclude updating challenges for deactivated users and to not attest for deactivated users
  • AUD-1117 Updates identity to not send notifications to users who have been deactivated

Tests

❗ TODO ❗

Testing thoughts:

  • Tracks without segments throw warnings when updated from the client. But we can't edit deleted/deactivated account tracks
  • Double check that the lack of download CID combined with is_downloadable: true doesn't have any odd behavior
  • How to test rewards scenario?
  • How to test notifications scenario?

Note: Adds queries to get users in some places, which increases overhead.

How will this change be monitored?

@rickyrombo rickyrombo added discovery-node Discovery Node (previously known as Discovery Provider) identity-service Identity Service labels Dec 13, 2021
raymondjacobson
raymondjacobson approved these changes Dec 15, 2021
View reviewed changes
discovery-provider/src/api/v1/tracks.py
"id": [decoded_id],
"with_users": True,
}
tracks = get_tracks(args)
Copy link
Member

Choose a reason for hiding this comment

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

i think the extra latency here would be fine. i like this solution (just musing aloud)

discovery-provider/src/queries/get_attestation.py Show resolved Hide resolved
@rickyrombo rickyrombo marked this pull request as ready for review December 15, 2021 18:29
@rickyrombo rickyrombo merged commit 71d11d7 into master Dec 21, 2021
@rickyrombo rickyrombo deleted the mjp-deactivated branch December 21, 2021 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@piazzatron piazzatron piazzatron approved these changes

@raymondjacobson raymondjacobson raymondjacobson approved these changes

Assignees

@piazzatron piazzatron

Labels
discovery-node Discovery Node (previously known as Discovery Provider) identity-service Identity Service size/M
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@rickyrombo @raymondjacobson @piazzatron