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

[HOLD for payment 2022-04-13] [$750] When clicking on Room Settings after workspace deleted, error displayed - reported by @Santhosh-Sellavel #8033

Closed
mvtglobally opened this issue Mar 8, 2022 · 55 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor

Comments

@mvtglobally
Copy link

mvtglobally commented Mar 8, 2022

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. Delete workspace
  2. Open #announce room or any room
  3. Click the header to view the details page
  4. Click on settings
  5. Error page is shown on web/Desktop app.

Expected Result:

Open Settings Page for the room

Actual Result:

Error Page Appeared

Workaround:

unknown

Platform:

Where is this issue occurring?

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

Version Number: 1.1.40-1
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

https://user-images.githubusercontent.com/43995119/157184784-9e5bedd9-107d-4c3d-a8b4-97c0ca2bb4e0.mov
Screenshot 2022-02-25 at 3 01 37 AM

Upwork job URL: https://www.upwork.com/jobs/~01c95cafe702e5684f
Issue reported by: @Santhosh-Sellavel
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1645738088150939

View all open jobs on GitHub

@MelvinBot
Copy link

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

@alex-mechler
Copy link
Contributor

This is certainly something that we should handle better. Sending external

@alex-mechler alex-mechler removed their assignment Mar 8, 2022
@alex-mechler alex-mechler added the External Added to denote the issue can be worked on by a contributor label Mar 8, 2022
@MelvinBot
Copy link

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

@MelvinBot
Copy link

Triggered auto assignment to Contributor-plus team member for initial proposal review - @parasharrajat (Exported)

@MelvinBot MelvinBot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Mar 10, 2022
@MelvinBot
Copy link

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

@trjExpensify
Copy link
Contributor

Ah, I didn't catch this! Upwork job here.

@MelvinBot MelvinBot removed the Overdue label Mar 10, 2022
@ahmdshrif
Copy link
Contributor

ahmdshrif commented Mar 10, 2022

when i delete the woarkspace the #announce room is deleted and settings is disappear.

Screen Shot 2022-03-10 at 10 09 51 PM

@trjExpensify
Copy link
Contributor

Oh, hm.. @TomatoToaster has this been fixed then? CC: @Santhosh-Sellavel

@ahmdshrif
Copy link
Contributor

not really, I have an old account which I delete the workspace before this was solved.
the result i still have setting on #announce on the old one and if I open it, it will crash the app.

so I think this issue will appear on developer accounts only they delete workspace on development mode before this gets fixed.

@mollfpr
Copy link
Contributor

mollfpr commented Mar 13, 2022

The problem does not appear in the latest main branch. But the code that will make this happen again is still there.

Proposal

This happen because the object policy have posibility to return null. We need add some condition to check if policy null we return false.

const linkedWorkspace = _.find(this.props.policies, policy => policy.id === this.props.report.policyID);

-         const linkedWorkspace = _.find(this.props.policies, policy => policy.id === this.props.report.policyID);

+        const linkedWorkspace = _.find(this.props.policies, (policy) => {
+            if (policy === null) {
+                return false;
+            }
+
+            return policy.id === this.props.report.policyID;
 +       });

Before changes

Screen.Recording.2022-03-13.at.11.47.40.mov

After changes

Screen.Recording.2022-03-13.at.11.48.07.mov

@parasharrajat
Copy link
Member

parasharrajat commented Mar 13, 2022

We mostly try to fix the root cause of the problem before patching the code. I will wait to see more proposals that can identify why this line creates an issue and works sometimes. This issue has been reported a few times earlier and this line creates the issue but why? And which policies have null data.

@mollfpr
Copy link
Contributor

mollfpr commented Mar 14, 2022

@parasharrajat It's because there are some policy from onyx which ONYXKEYS.COLLECTION.POLICY has some key with a null value. The null value could be from deleting the policy.

function deletePolicy(policyID = '', shouldAutomaticallyReroute = true) {
return API.Policy_Delete({policyID})
.then((response) => {
if (response.jsonCode !== 200) {
// Show the user feedback
const errorMessage = translateLocal('workspace.new.genericFailureMessage');
Growl.error(errorMessage, 5000);
return;
}
// Removing the workspace data from Onyx as well
return Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, null);

Screen Shot 2022-03-14 at 16 59 27

@parasharrajat
Copy link
Member

Ok, I see. Did you run some actions which resulted in this Onyx state or does it already has null values?

But that said, I have already faced this issue earlier and I know that #8033 (comment) fixes this crash as suggested earlier https://expensify.slack.com/archives/C01GTK53T8Q/p1643191135467000

I would be curious to know why are we storing null values in the Onyx. that seems like the real issue.

@parasharrajat
Copy link
Member

Also, I like to point out that I reported this issue a month back from this issue https://expensify.slack.com/archives/C01GTK53T8Q/p1643191135467000. And there was a +1 on that. I don't why it wasn't ported to GH. cc: @trjExpensify

@trjExpensify
Copy link
Contributor

Looking at that original convo @parasharrajat, Applause and Aldo weren't able to reproduce it with the steps provided?

@Santhosh-Sellavel
Copy link
Collaborator

Santhosh-Sellavel commented Apr 1, 2022

I am still in favor of assigning this job to @mollfpr. Also, PR is already created and approved.

@parasharrajat, I too think the job should be assigned to @mollfpr because that's the only fair thing to do. We will discuss this further internally in the upcoming days. How to address this kind of situation in the future, thanks!

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Apr 6, 2022
@melvin-bot melvin-bot bot changed the title [$750] When clicking on Room Settings after workspace deleted, error displayed - reported by @Santhosh-Sellavel [HOLD for payment 2022-04-13] [$750] When clicking on Room Settings after workspace deleted, error displayed - reported by @Santhosh-Sellavel Apr 6, 2022
@melvin-bot
Copy link

melvin-bot bot commented Apr 6, 2022

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.1.51-0 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2022-04-13. 🎊

@trjExpensify trjExpensify removed their assignment Apr 8, 2022
@trjExpensify trjExpensify added External Added to denote the issue can be worked on by a contributor and removed External Added to denote the issue can be worked on by a contributor labels Apr 8, 2022
@melvin-bot
Copy link

melvin-bot bot commented Apr 8, 2022

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

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Apr 8, 2022
@trjExpensify trjExpensify added External Added to denote the issue can be worked on by a contributor and removed External Added to denote the issue can be worked on by a contributor labels Apr 8, 2022
@melvin-bot
Copy link

melvin-bot bot commented Apr 8, 2022

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

@trjExpensify
Copy link
Contributor

👋 I'm OoO next week (so is @laurenreidexpensify, I believe), so reassigning for another CM to handle the payment after the regression period. Thanks, @dylanexpensify!

Upwork job is here: https://www.upwork.com/jobs/~01c95cafe702e5684f

@melvin-bot melvin-bot bot added the Overdue label Apr 11, 2022
@dylanexpensify
Copy link
Contributor

You got it tom!

@melvin-bot melvin-bot bot added Daily KSv2 and removed Overdue Daily KSv2 labels Apr 11, 2022
@dylanexpensify
Copy link
Contributor

@parasharrajat sent offer to accept and pay you out as C+!

@Santhosh-Sellavel please apply to this job to be paid for reporting the issue!

@dylanexpensify
Copy link
Contributor

@mollfpr payment sent, contract ended!

@Santhosh-Sellavel
Copy link
Collaborator

@dylanexpensify done!

@dylanexpensify
Copy link
Contributor

Offer sent @Santhosh-Sellavel!

@dylanexpensify
Copy link
Contributor

Alrighty! Everyone has been paid, contracts ended, posts removed! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor
Projects
None yet
Development

No branches or pull requests