WP-104: phantom entry in ViewTab when user removed

[van-go]

Overview

When a user was selected in the View Team tab, then the same user was removed in the Manage Team tab, when you clicked back to View Team, that user's info was still here, even if they were removed.

Related

• WP-104

Changes

Added a conditional statement to reset the AllocationContactCard back to it's default state where it displays "Click on a user’s name to view their allocation usage." when the tab is switched back to View Team from Manage Team.

Testing

Find a Allocation that your are PI or manager on (you'll need to see the Manage Team tab)

1. In the View Team tab, select a user
2. Click on Manage Team
3. From the Manage Team tab, remove the previously selected user.
4. Click on View Team
5. Notice that the previously selected user info is not there. Instead it should display "Click on a user’s name to view their allocation usage."

UI

Screen.Recording.2023-10-10.at.1.24.49.PM.mov

Notes

[codecov]

Codecov Report

Merging #884 (ada19e6) into main (5e6be96) will increase coverage by 6.34%.
The diff coverage is 60.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #884      +/-   ##
==========================================
+ Coverage   63.41%   69.75%   +6.34%     
==========================================
  Files         432      224     -208     
  Lines       12388     6256    -6132     
  Branches     2579     1877     -702     
==========================================
- Hits         7856     4364    -3492     
+ Misses       4322     1814    -2508     
+ Partials      210       78     -132     

Flag	Coverage Δ
javascript	69.75% <60.00%> (-0.01%)	⬇️
unittests	?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
...ocationsTeamViewModal/AllocationsTeamViewModal.jsx	65.15% <60.00%> (-0.43%)	⬇️

... and 208 files with indirect coverage changes

[chandra-tacc]

@van-go
I cannot test this because of my role, trying to write unit test.
The side-effect of this is selection is never preserved when moving between tabs. Is that what we want ?
Also, there is a lint error.

[van-go]

@chandra-tacc I thought about the fact that the selection is not preserved when moving between tabs, but I don't think it's a real concern. At least it's not as big of a concern as showing a removed user. But, if there's a different approach you suggest, I'll look into implementing it. Also, I'll look at and fix the linting issue.

[chandra-tacc]

@chandra-tacc I thought about the fact that the selection is not preserved when moving between tabs, but I don't think it's a real concern. At least it's not as big of a concern as showing a removed user. But, if there's a different approach you suggest, I'll look into implementing it. Also, I'll look at and fix the linting issue.

@van-go - This definitely will fix the bug. I'm not sure about stats on usage, if deletion of user is common vs other non-deletion related tasks.

If we want to preserve selection, one way to do that is check if the selected user exists in the list.

(untested):

const selectedCardExists = card && !isLoading && projectId in teams && teams[projectId].some(user => user.username === card.username);
if (!selectedCardExists) setCard(null);

Also, is there is a trick to test this locally without being a PI like role?
I started unit test, but it is going to take longer

[chandra-tacc]

comments added here: #884 (comment)

[chandra-tacc]

Thanks for adjusting to the user scenario. The test case works, but the code needs adjustment. I shared the details in the code comments.

[chandra-tacc]

The useState usage is incorrect. It should be

const [selectedUser, setSelectedUser] = useState(null);

Otherwise selectedUser is a tuple of [null, setter].
When it is set in line 89, it simply overrides the value with username.

A reason all of this is still working is because in line 88, if condition is checking an object removingUserOperation, who value in cases where user is not removed will be

{
   "loading": false,
   "error": false,
   "username": ""
}

So, for all non-deletion scenarios, selectedUser is always empty string.
And in line 91, the condition fails and setCard(null) will not execute.

In delete user scenario:

• removingUserOperation will be

{
    "loading": false,
    "error": false,
    "username": "foobar"
}

selectedUser will be "foobar"
line 91 check passes
and setCard is null.

The test cases work, but the code needs cleanup:
line 87, useState is not needed, just initialize to null
line 88, check should be checking if removingUserOperation && removingUserOperation.username is not empty.
Rest of the code will work as is.

[chandra-tacc]

LGTM! Thanks for adjustments.