Skip to content

fix: [M3-8158] - Render the correct Avatar icon for ACTIONS_WITHOUT_USERNAMES#10923

Merged
mjac0bs merged 3 commits intolinode:developfrom
mjac0bs:M3-8158-fix-actions-without-usernames-avatar
Sep 12, 2024
Merged

fix: [M3-8158] - Render the correct Avatar icon for ACTIONS_WITHOUT_USERNAMES#10923
mjac0bs merged 3 commits intolinode:developfrom
mjac0bs:M3-8158-fix-actions-without-usernames-avatar

Conversation

@mjac0bs
Copy link
Contributor

@mjac0bs mjac0bs commented Sep 11, 2024

Description 📝

One more bug fix...

There are a very small number of events that we consider ACTIONS_WITHOUT_USERNAMES. Some of these actions have no username, and other we want to override the username with Linode - we have a util called getEventUsername to do this for us.

We weren't making use of the username from the util correctly in this component, which we assign at L36.

Without using this util, if a username existed (as it does for entity_transfer_create events), we were rendering the wrong initial in the avatar component. Since this is an 'action without username', we want the username to be Linode and to render the Akamai logo as the avatar.

We were already using this util in the EventRow component on Event Landing page; the bug was just in the Notification Center event.

No changeset, didn't make it to prod. Adding a changeset because this actually was an existing bug in prod, but becomes more obvious once we start using the initialed avatars.

Changes 🔄

  • Uses the username from getEventUsername util to correctly identify events whose usernames we are changing and render the correct avatar accordingly.

Target release date 🗓️

9/16/24

Preview 📷

Before After
Screenshot 2024-09-11 at 12 35 04 PM Screenshot 2024-09-11 at 12 34 19 PM
Prod: image Screenshot 2024-09-11 at 1 25 00 PM

How to test 🧪

Prerequisites

(How to setup test environment)

  • Use an account without a Gravatar.
  • In dev, trigger a service transfer.

Reproduction steps

(How to reproduce the issue, if applicable)

  • Open the notification center.
  • Notice that although the username associated with the service transfer create event shows up as 'Linode' in the notification, the avatar has the first letter of your account username. (This is because the event.username is actually your user when coming from the API.)

Verification steps

(How to verify changes)

  • Check out this PR.
  • Open the notification center.
  • Notice that by using the util, we now are correctly overriding that event.username with Linode when passing that prop to the Avatar component. The Akamai logo is displayed as a result.
  • Verify that that matches the event in Events Landing.

As an Author I have considered 🤔

Check all that apply

  • 👀 Doing a self review
  • ❔ Our contribution guidelines
  • 🤏 Splitting feature into small PRs
  • ➕ Adding a changeset
  • 🧪 Providing/Improving test coverage
  • 🔐 Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • 👣 Providing comprehensive reproduction steps
  • 📑 Providing or updating our documentation
  • 🕛 Scheduling a pair reviewing session
  • 📱 Providing mobile support
  • ♿ Providing accessibility support

@mjac0bs mjac0bs self-assigned this Sep 11, 2024
@mjac0bs mjac0bs requested a review from a team as a code owner September 11, 2024 20:02
@mjac0bs mjac0bs requested review from bnussman-akamai and coliu-akamai and removed request for a team September 11, 2024 20:02
@github-actions
Copy link

github-actions bot commented Sep 11, 2024

Coverage Report:
Base Coverage: 86.64%
Current Coverage: 86.64%

@mjac0bs mjac0bs added Add'tl Approval Needed Waiting on another approval! Bug Fixes for regressions or bugs labels Sep 11, 2024
Copy link
Contributor

@coliu-akamai coliu-akamai left a comment

Choose a reason for hiding this comment

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

thanks Mariah! saw the bug on develop, confirmed bug is fixed in this branch 🎉

@coliu-akamai coliu-akamai added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Sep 11, 2024
@mjac0bs mjac0bs merged commit 7ad5ec2 into linode:develop Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Approved Multiple approvals and ready to merge! Bug Fixes for regressions or bugs

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants