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

Workspace - Fields appear twice in invite people page #4060

Closed
kavimuru opened this issue Jul 15, 2021 · 17 comments
Closed

Workspace - Fields appear twice in invite people page #4060

kavimuru opened this issue Jul 15, 2021 · 17 comments
Assignees
Labels
DeployBlockerCash This issue or pull request should block deployment Engineering Hourly KSv2

Comments

@kavimuru
Copy link

kavimuru commented Jul 15, 2021

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. Launch the app
  2. Log in with expensifail account
  3. Click on Green FAB
  4. Select New Workplace
  5. Choose the name and click Get Started
  6. Click on People and click Invite

Expected Result:

Fields appear once in the Invite people page.

Actual Result:

Fields appear twice in the Invite people page.

Workaround:

Unknown

Platform:

Where is this issue occurring?

Web ✔️
iOS ✔️
Android ✔️
Desktop App ✔️
Mobile Web ✔️

Version Number:
1.0.78-0
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

Expensify/Expensify Issue URL:

Bug5152196_Screen_Recording_20210714-200845_Expensifycash.mp4

View all open jobs on Upwork

@kavimuru kavimuru added the DeployBlockerCash This issue or pull request should block deployment label Jul 15, 2021
@OSBotify
Copy link
Contributor

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open StagingDeployCash deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

@MelvinBot
Copy link

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

@tgolen
Copy link
Contributor

tgolen commented Jul 15, 2021

Hm, that's pretty weird, but I verified it happens! Must be rendered twice or something. Gonna try to look into this

@tgolen
Copy link
Contributor

tgolen commented Jul 15, 2021

OK, I'm trying to debug this locally, but I am not able to create a workspace for some reason. I keep getting the error Your session has expired. Please sign in again. from the API and having trouble getting this working.

@tgolen
Copy link
Contributor

tgolen commented Jul 15, 2021

All right, signing out of e.cash and signing back in seemed to have fixed it.

The problem is, I can't reproduce the problem locally:

image

I'll try checking out staging to see if I can reproduce it with that code

@tgolen
Copy link
Contributor

tgolen commented Jul 15, 2021

OK, so if I check out the staging branch locally, I can reproduce the bug.

However, if I apply the changes from #3988 to the staging branch, then the bug is fixed.

What's odd is that the PR says it's on staging, but that doesn't seem possible.

@tgolen
Copy link
Contributor

tgolen commented Jul 15, 2021

I'm working with @roryabraham to see if we can determine what went wrong. You can see in this PR #4083 the difference between main and staging, when there should be no difference.

@tgolen
Copy link
Contributor

tgolen commented Jul 15, 2021

Not really an update, but we are waiting to do a full deploy from main to staging to see if it fixes this.

@isagoico
Copy link

@tgolen We retested this on latest build (1.0.79-0) and it's still reproducible 😟

@tgolen
Copy link
Contributor

tgolen commented Jul 16, 2021

@roryabraham Do you have any update for this?

@parasharrajat
Copy link
Member

parasharrajat commented Jul 16, 2021

@tgolen I just checked the code for the page. It seems the merge Conflict was not resolved correctly. Both old and new changes are merged into the code which is causing duplicates. It could be me as well.

Lol. I think it's me. https://github.com/Expensify/Expensify.cash/blame/staging/src/pages/workspace/WorkspaceInvitePage.js

But it is very strange to see that main does not have this duplicate code.

Edit:
Not me I checked my local.

@roryabraham
Copy link
Contributor

No update other than that this diff has the same weirdness we observed before. Unless other PRs have been merged since the lock label was added, there should be no diff between main and the latest tag.

@tgolen
Copy link
Contributor

tgolen commented Jul 16, 2021

OK, I'm going to set this back to daily since we're just waiting on another deploy from main to staging to see if it clears up.

@roryabraham
Copy link
Contributor

Well, there has been another deploy from main to staging, but for some reason it's still not registering the diff we expect. This is the code that updates staging from main. If we look at the logs for the latest run, we can see that, when main is merged into the temporary copy of staging, WorkspaceInvitePage is not included in the list of changed files. The pull request that was created for this run is right here, and I would've expected it to include the changes in WorkspaceInvitePage from main.

Because I'm unsure why this happened and we've never seen it happen before, let's open a PR directly from main to staging and merge that. It won't cause a staging deploy, so we'll have to unlock and relock the StagingDeployCash in order to trigger another staging deploy.

@parasharrajat
Copy link
Member

Maybe we can modify the invite page and it will be included and there is an pr #4068 here.

@roryabraham
Copy link
Contributor

Maybe we can modify the invite page and it will be included and there is an pr #4068 here.

Well, I can't explain why the existing diff from the WorkspaceInvitePage is not being included in the PR that's supposed to update main from staging, so I'm not sure that merging more changes to main will solve the problem. It might, but I don't know.

Instead, I think we should just merge #4113 and then unlock and relock the StagingDeployCash.

@botify botify closed this as completed Jul 16, 2021
@tgolen
Copy link
Contributor

tgolen commented Jul 16, 2021

Not fixed! That PR merged staging into main... so main is broken again. I'll submit a PR to fix main and we can CP it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DeployBlockerCash This issue or pull request should block deployment Engineering Hourly KSv2
Projects
None yet
Development

No branches or pull requests

8 participants