Skip to content

Add organization avatars#4818

Merged
dcramer merged 11 commits intomasterfrom
org-avatars
Jan 26, 2017
Merged

Add organization avatars#4818
dcramer merged 11 commits intomasterfrom
org-avatars

Conversation

@dcramer
Copy link
Copy Markdown
Member

@dcramer dcramer commented Jan 24, 2017 •
edited
Loading

  • Add OrganizationAvatar model
  • Refactor shared avatar components
  • Add avatar/avatarType to organization details endpoint

@getsentry-bot
Copy link
Copy Markdown
Contributor

getsentry-bot commented Jan 24, 2017 •
edited
Loading

2 Warnings
⚠️ Changes require @getsentry/security sign-off
⚠️ Big PR – consider splitting it up into multiple changesets

Security concerns found

  • src/sentry/api/endpoints/organization_details.py

Generated by 🚫 danger

@dcramer
Copy link
Copy Markdown
Member Author

dcramer commented Jan 24, 2017

Can follow up with UI in another patch to avoid too many changes at once

@dcramer dcramer requested a review from macqueen January 24, 2017 21:30
dcramer
dcramer commented Jan 24, 2017
View reviewed changes
Comment thread src/sentry/api/endpoints/useravatar.py Outdated
Copy link
Copy Markdown
Member Author

@dcramer dcramer Jan 24, 2017

Choose a reason for hiding this comment

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

ill plan to refactor this to conform to API naming and put it in the UserDetails endpoint in a followup patch

macqueen
macqueen approved these changes Jan 26, 2017
View reviewed changes
Copy link
Copy Markdown
Contributor

@macqueen macqueen left a comment

Choose a reason for hiding this comment

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

left a couple comments, but otherwise 👍

Comment thread src/sentry/api/fields/avatar.py Outdated
Copy link
Copy Markdown
Contributor

@macqueen macqueen Jan 26, 2017

Choose a reason for hiding this comment

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

can these commented out lines be deleted?

Comment thread src/sentry/models/avatar.py Outdated
Copy link
Copy Markdown
Contributor

@macqueen macqueen Jan 26, 2017

Choose a reason for hiding this comment

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

this needs to be updated to ALLOWED_SIZES = (20, 32, 36, 48, 52, 64, 80, 96, 120) i think. (i think i added 36 as an allowed size to fix a UI bug since initially doing this model)

Comment thread src/sentry/south_migrations/0287_auto__add_organizationavatar.py Outdated
Copy link
Copy Markdown
Contributor

@macqueen macqueen Jan 26, 2017

Choose a reason for hiding this comment

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

there are 2 additional migrations in master now, so i think you need to regenerate this (or however else you fix it)

@dcramer dcramer merged commit 11a1280 into master Jan 26, 2017
@dcramer dcramer deleted the org-avatars branch January 26, 2017 19:30
@Belax8 Belax8 mentioned this pull request Aug 15, 2017
Closed
@github-actions github-actions Bot locked and limited conversation to collaborators Dec 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@macqueen macqueen macqueen approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Security

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dcramer @getsentry-bot @macqueen