Skip to content

linkify all user names on ratings list; handle an account response with undefined fields #13503

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

Merged
merged 2 commits into from
Mar 26, 2025

Conversation

eviljeff
Copy link
Member

@eviljeff eviljeff commented Mar 19, 2025

Fixes mozilla/addons#15403

Shows user account links for all ratings, and hides the fields that won't be present on the user profile page.

profile page

before
Screenshot 2025-03-19 121902
(before mozilla/addons#15402 changed the API it would have been a 404)

after
Screenshot 2025-03-19 121846

ratings

before
Screenshot 2025-03-19 122209
after
Screenshot 2025-03-19 122229

@eviljeff
Copy link
Member Author

re: covecov error - I've looked but I'm not sure where I'd be adding tests, exactly (codecov claims some of the existing lines in getFormValues are covered, but can't see where). The changes in getFormValues were only added to stop Flow from complaining about the values being potentially undefined anyway - in reality your own account response will always contain those object keys, as now.

@eviljeff eviljeff requested review from a team and diox and removed request for a team March 19, 2025 12:39
@eviljeff
Copy link
Member Author

Note, the amo api on dev and stage is supposed to be returning an account response that includes all the fields, so it shouldn't be possible to test this with yarn amo:dev, but good news! (as I write) the API_GATE isn't working 🙃 .

@diox
Copy link
Member

diox commented Mar 20, 2025

re: covecov error - I've looked but I'm not sure where I'd be adding tests, exactly (codecov claims some of the existing lines in getFormValues are covered, but can't see where). The changes in getFormValues were only added to stop Flow from complaining about the values being potentially undefined anyway - in reality your own account response will always contain those object keys, as now.

AFAICT you have no tests that simulate biography, occupation, location, homepage etc being undefined when going through UserProfileEdit page. All tests in TestUserProfileEdit.js use createUserAccountResponse() either directly or through dispatchSignInActionsWithStore(), without setting those fields to undefined, so they get the defaults defined in that function. So I'd say codecov complaining about lack of coverage here is warranted.

Copy link
Member

@diox diox left a comment

Choose a reason for hiding this comment

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

On top of my comment about coverage...

I'm currently seeing, with dev API:
Screenshot 2025-03-21 at 16-09-17 User Profile for Mozilla QA(test_user3) – Add-ons for Firefox (en-US)

Which doesn't match your "after" screenshot.

@eviljeff
Copy link
Member Author

The after is what it will look like once the shim is disabled for v5 - with the shim enabled some of the fields are still returned.

i think I'll disable the shim for v5 (different ticket) and update when that's live

Copy link

codecov bot commented Mar 25, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.28%. Comparing base (8e6ffaa) to head (f9a6e95).
Report is 20 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #13503   +/-   ##
=======================================
  Coverage   98.28%   98.28%           
=======================================
  Files         267      267           
  Lines       10610    10617    +7     
  Branches     3233     3240    +7     
=======================================
+ Hits        10428    10435    +7     
  Misses        169      169           
  Partials       13       13           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@eviljeff eviljeff requested a review from diox March 25, 2025 11:33
@eviljeff
Copy link
Member Author

mozilla/addons-server#23223 removes the shim for v5 - the testing will be accurate after that PR is merged and on addons-dev

@eviljeff eviljeff merged commit b0b2b89 into master Mar 26, 2025
11 checks passed
@eviljeff eviljeff deleted the 15403-handle-minimal-non-developer-profiles branch March 26, 2025 14:15
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.

[Task]: Update frontend to handle non-developer profiles
2 participants