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

add users.avatar_url constraint and default #1141

Merged
merged 4 commits into from
Aug 13, 2024

Conversation

CollinBeczak
Copy link
Contributor

Resolved conversation from #1137
Resolves issue: Resolves: maproulette/maproulette3#2205

This pr adds a new migration file that sets the default of the users.avatar_url column to '/assets/images/user_no_image.png', and prevents its value from being null. This change made it so that avatar_url was no longer an option, so tests and parserss were updated accordingly.

@CollinBeczak CollinBeczak marked this pull request as ready for review August 12, 2024 03:52
Copy link
Contributor

@ljdelight ljdelight left a comment

Choose a reason for hiding this comment

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

I see unintentional changes to Groups and other objects, so I think we should remove about all of the scala changes related to the avatar_url db requirement changes. For example, it's fine to update user.avatar_url parsers but no changes should be done for group.avatar_url (which looks to always be null).

@CollinBeczak CollinBeczak force-pushed the collinBeczak/add-avatar_url-contraint-and-default branch from b6b5066 to 622f9af Compare August 13, 2024 22:55
Copy link

sonarcloud bot commented Aug 13, 2024

@ljdelight ljdelight self-requested a review August 13, 2024 23:25
Copy link
Contributor

@ljdelight ljdelight left a comment

Choose a reason for hiding this comment

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

LGTM

@ljdelight ljdelight merged commit 2a2e18c into main Aug 13, 2024
11 checks passed
@ljdelight ljdelight deleted the collinBeczak/add-avatar_url-contraint-and-default branch August 13, 2024 23:26
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.

Users with null users.avatar_url Causes Error in GET /api/v2/challenge/1/comments Endpoint
2 participants