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

Use MinimalUserProfileSerializer for AccountViewSet for non-developers #23192

Conversation

eviljeff
Copy link
Member

@eviljeff eviljeff commented Mar 19, 2025

Fixes: mozilla/addons#15479 - follow up for mozilla/addons#15402, fixing a bug in #23154

Description

See issue. We were using the wrong serializer that didn't have the API_GATE integration to shim the fields.

Context

Testing

access any /api/v[3|4|5]/accounts/account/(int:user_id)/ url without auth; see all the fields present, but with biography, etc (see original issue and pr) shim'd as null rather than omitted.

Checklist

  • Add #ISSUENUM at the top of your PR to an existing open issue in the mozilla/addons repository.
  • Successfully verified the change locally.
  • The change is covered by automated tests, or otherwise indicated why doing so is unnecessary/impossible.
  • Add before and after screenshots (Only for changes that impact the UI).
  • Add or update relevant docs reflecting the changes made.

@eviljeff eviljeff marked this pull request as ready for review March 19, 2025 16:08
@eviljeff eviljeff requested review from a team and KevinMind and removed request for a team March 19, 2025 16:16
@@ -1268,7 +1268,28 @@ def test_view_deleted(self):
response = self.client.get(self.url)
assert response.status_code == 404

def test_is_not_full_public_profile_because_not_developer(self):
def test_is_not_full_public_profile_because_not_developer_but_fields_present(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I would rather this be two different tests, but since we are trying to cherry-pick no need.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is two tests:

  • test_is_not_full_public_profile_because_not_developer_but_fields_present tests the case with the shim
  • test_is_not_full_public_profile_because_not_developer_no_fields tests without the shim is, but as it's identical to the original test_is_not_full_public_profile_because_not_developer it doesn't show any changed lines

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean the 2 sets of assertions in the first test.

@KevinMind KevinMind self-requested a review March 19, 2025 16:32
Copy link
Contributor

@KevinMind KevinMind left a comment

Choose a reason for hiding this comment

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

URL: http://olympia.test/api/v3/accounts/account/1/

Response:

{
  "id": 1,
  "name": "Test",
  "url": "http://olympia.test/en-US/firefox/user/1/",
  "username": "local_admin",
  "average_addon_rating": null,
  "created": "2025-03-03T11:06:15Z",
  "biography": null,
  "has_anonymous_display_name": false,
  "has_anonymous_username": false,
  "homepage": null,
  "is_addon_developer": true,
  "is_artist": true,
  "location": null,
  "occupation": null,
  "num_addons_listed": 0,
  "picture_type": null,
  "picture_url": null
}

URL: http://olympia.test/api/v4/accounts/account/1/

{
  "id": 1,
  "name": "Test",
  "url": "http://olympia.test/en-US/firefox/user/1/",
  "username": "local_admin",
  "average_addon_rating": null,
  "created": "2025-03-03T11:06:15Z",
  "biography": null,
  "has_anonymous_display_name": false,
  "has_anonymous_username": false,
  "homepage": null,
  "is_addon_developer": true,
  "is_artist": true,
  "location": null,
  "occupation": null,
  "num_addons_listed": 0,
  "picture_type": null,
  "picture_url": null
}

Note

only on v5 I got an error when using an invalid sessionID cookie

URL: http://olympia.test/api/v5/accounts/account/1/

{
  "id": 1,
  "name": "Test",
  "url": "http://olympia.test/en-US/firefox/user/1/",
  "username": "local_admin",
  "average_addon_rating": null,
  "created": "2025-03-03T11:06:15Z",
  "biography": null,
  "has_anonymous_display_name": false,
  "has_anonymous_username": false,
  "homepage": null,
  "is_addon_developer": true,
  "is_artist": true,
  "location": null,
  "occupation": null,
  "num_addons_listed": 0,
  "picture_type": null,
  "picture_url": null
}

@eviljeff
Copy link
Member Author

only on v5 I got an error when using an invalid sessionID cookie

@KevinMind sounds like something that needs further investigation (but not related to this change)

@eviljeff eviljeff merged commit d7c263e into mozilla:master Mar 19, 2025
41 checks passed
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.

[Bug]: non-developer user account responses are omitting fields that should have been frozen in the API
2 participants