[$250] Red dot is missing on workspace settings page

[mvtglobally]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

1. Do Onyx.merge('policy_8C6BF17C3E92D600', {pendingAction: 'add', errorFields: {avatar: {1: 'This is an error'}}}); in the console
2. Notice red dot on your profile image, click it
3. Notice red dot on your workspace name, click it
4. Issue: there is no RBR in the workspace settings
5. Click General Settings and see the red dot by the avatar

Expected Result:

RBR should display on workspace settings page

Actual Result:

RBR is missing on workspace settings page

Workaround:

unknown

Platform:

Where is this issue occurring?

• Web
• iOS
• Android
• Desktop App
• Mobile Web

Version Number: 1.1.96-0
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

Upwork job URL: https://www.upwork.com/jobs/~014435583ac8f1a5c4
Issue reported by: @puneetlath
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1662106163497789

View all open jobs on GitHub

[melvin-bot]

Triggered auto assignment to @danieldoglas (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[thesahindia]

Proposal

We are checking wrong key for errors ( avatarURL )

App/src/pages/workspace/WorkspaceInitialPage.js

Line 85 in a944496

|| !_.isEmpty(lodashGet(this.props.policy, 'errorFields.avatarURL', {}));

We have to replace it with avatar

[danieldoglas]

@mvtglobally the API is actually returning avatarURL and not avatar. Can you point to me where you got that testing steps from? We'll probably need to update it

[puneetlath]

@danieldoglas that was from me when I was testing a different PR. So if I had that wrong, then we're all good.

[danieldoglas]

Got it! so I think we can close this one out! thanks @puneetlath

[thesahindia]

Hi @danieldoglas, I think we should reopen this issue.

Steps to repro the issue -

1. Add a photo in your workspace "xyz"
2. Open a new tab and visit https://www.expensify.com/admin_policies and delete that "xyz" workspace
3. Go back to the tab where new dot is open and try removing the image from "xyz" workspace
4. Check you see an error
5. Now press back and check if you are seeing the red indicator

We get the avatar related errors at errorFields.avatar but we are checking for the errors at errorFields.avatarURL we can solve this by replacing avatarURL with avatar as proposed here.

[thesahindia]

Here's a video -

Screen.Recording.2022-09-07.at.3.51.11.PM.mov

[danieldoglas]

@thesahindia @puneetlath it seems like we're using avatar and avatarURL, depending on the requests. So it was my mistake here on closing this.

in this specific case, it seems like we should check for both, so I would adjust your proposal to do that, @thesahindia. I'll export this.

At the same time, tagging @Gonals to give his opinion on standardizing the name for avatar/avatarURL in all methods.

[melvin-bot]

Triggered auto assignment to @michaelhaxhiu (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[danieldoglas]

Also, this situation only happens because the Workspace was delete before editing. I think we're not doing the right requests here to guarantee that we're actually using a valid workspace when accessing it. @arosiclair might be able to give more context upon this.

[thesahindia]

I thought we only get the error at avatar and not at avatarURL because here we are only checking avatar key for the error. If we also get the error at avatarURL then I think we will have to make the same change to check both keys here too

App/src/pages/workspace/WorkspaceSettingsPage.js

Line 106 in c8018b2

errors={lodashGet(this.props.policy, 'errorFields.avatar', null)}

[Gonals]

I like avatarURL, but I'm fine with either

[arosiclair]

Also, this situation only happens because the Workspace was delete before editing. I think we're not doing the right requests here to guarantee that we're actually using a valid workspace when accessing it.

I don't think we get Pusher updates from OldDot so the policy doesn't get removed in NewDot until you refresh (OpenApp / ReconnectApp commands). If we're offline, the policy will stay until you reconnect.

[danieldoglas]

@arosiclair When we open a policy(workspace) page, we're not doing a GET for that policy today?

[arosiclair]

@arosiclair When we open a policy(workspace) page, we're not doing a GET for that policy today?

We are until this PR I'm working on get's merged

[thesahindia]

@thesahindia @puneetlath it seems like we're using avatar and avatarURL, depending on the requests. So it was my mistake here on closing this.

@danieldoglas, are there any steps to repro an error for avatarURL?

[danieldoglas]

@thesahindia No, just for avatar. It is already treating avatarURL, as seen here, so we shouldn't have any errors.

[thesahindia]

I was trying to produce an error for avatarURL key, I wanted to check the general settings page when we get error at avatarURL because we aren't checking avatarURL for errors at general settings (Code)

So just to confirm, we will be adding another OR operator here

App/src/pages/workspace/WorkspaceSettingsPage.js

Line 106 in c8018b2

errors={lodashGet(this.props.policy, 'errorFields.avatar', null)}

and will check for errors at avatar !_.isEmpty(lodashGet(this.props.policy, 'errorFields.avatar, {}))

And we will be adding one OR operator here to also search for errors at avatarURL because we are only checking avatar for errors at this page

App/src/pages/workspace/WorkspaceSettingsPage.js

Line 106 in c8018b2

errors={lodashGet(this.props.policy, 'errorFields.avatar', null)}

[danieldoglas]

@thesahindia yes, that makes sense to me!

[melvin-bot]

Triggered auto assignment to @Julesssss (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[michaelhaxhiu]

@thesahindia nice catch on this GH and pushing to keep it open!

I just made the job in upwork here - https://www.upwork.com/jobs/~014435583ac8f1a5c4.

@danieldoglas is it safe to say we are hiring @thesahindia for the fix?

[danieldoglas]

@michaelhaxhiu for me yes, but @Julesssss has the final word in this case :D

[Julesssss]

Yeah, hiring @thesahindia

[melvin-bot]

📣 @thesahindia You have been assigned to this job by @Julesssss!
Please apply to this job in Upwork and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[Julesssss]

I also think we should merge the keys, but that's out of scope for this PR and requires backend changes.

[michaelhaxhiu]

@thesahindia invited you to job in case you missed this initially.

[thesahindia]

I also think we should merge the keys, but that's out of scope for this PR and requires backend changes.

Hey @Julesssss, I also believe we should merge the keys, I can wait for the backend change.

[thesahindia]

@thesahindia invited you to job in case you missed this initially.

Applied! Thanks.

[thesahindia]

Hey @Julesssss, I also believe we should merge the keys, I can wait for the backend change.

Let me know if we can wait for the backend change. If not I can raise the PR today.

cc: @Julesssss

[Julesssss]

@thesahindia let's forget about that for now and create a new refactor issue for merging the keys

[thesahindia]

#10706 has removed the usage of avatarURL so I think we don't need a PR now, right?

[Julesssss]

Ah yes. @michaelhaxhiu let's just pay out the job.

[thesahindia]

Ah yes. @michaelhaxhiu let's just pay out the job.

Hmm, I don't think it's eligible for the payment because no PR was created.

[Julesssss]

We spent some time here discussing, hence me leaning towards payment. I don't think it was obvious that a separate issue would make this one redundant.

[michaelhaxhiu]

Yep I agree with paying it @Julesssss. I think this was unforseen. All good with me proceeding rn?

[Julesssss]

Yep I agree with paying it @Julesssss. I think this was unforseen. All good with me proceeding rn?

Yeah 👍

[michaelhaxhiu]

done, closing ✌️

[thesahindia]

Thank you ♥️

[michaelhaxhiu]

weird, this didn't close initially when I thought it did.