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

Show participant is sharing with DSP when allow_sites is null #207

Merged
merged 5 commits into from
Oct 11, 2023

Conversation

jingyi-gao-ttd
Copy link
Contributor

@jingyi-gao-ttd jingyi-gao-ttd commented Oct 6, 2023

Description:

  • Show participant is sharing with DSP when allow_sites is null
  • Throw error when sharing participant /api/sharing/list/ return 404 (I believe we should hide sharing permission from menu for this case, but I think there is a separate ticket for this already.)

As discussed, when a participant is GENERATOR or SHARER or has a client_keypair, admin service creates a keyset for it. For now we don't allow user create keyset from portal

Required PR:

Manual tests

  1. Approve participant with a siteId that has SHAERE role, it should get empty allow_sites, and allow_types, it should show recommended bulk permission based on it's clientTypes
Screenshot 2023-10-06 at 3 56 56 pm
  1. Approve participant with a siteId with GENERATOR role, it should get null allow_sites and empty allow_types.
  • In the portal, it should show recommended bulk permission based on it's clientTypes
Screenshot 2023-10-06 at 3 59 58 pm
  • It should has allow_sites set to [] after click save in bulk permission
Screenshot 2023-10-06 at 4 00 38 pm
  1. Approve participant with a siteId with GENERATOR role, and setcompletedRecommendations to true from DB, it should show shared with DSP
Screenshot 2023-10-06 at 4 05 25 pm
  1. Approve participant with a siteId that has MAPPER role, it should error dialog when user goes to the sharing permission page

image

@jingyi-gao-ttd jingyi-gao-ttd marked this pull request as ready for review October 6, 2023 05:31
loadSharingList();
}, [loadSharingList]);
}, [throwError]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should provide a better error experience here - if there's a 404 from the admin service, we know exactly what's wrong. Just dumping the user to an error where they can't even see the menu any more isn't the right move.

setShowErrorDialog(true);
return;
}
throwError(e);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

if there is any other ApiError for this get request, I think we should still show error page

@lionell-pack-ttd
Copy link
Collaborator

I'm merging this as it's approved now, but Jingyi is unavailable at the moment.

@lionell-pack-ttd lionell-pack-ttd merged commit 5fb4823 into main Oct 11, 2023
2 checks passed
@lionell-pack-ttd lionell-pack-ttd deleted the jyg-UID2-1736-show-correct-sharing-types branch October 11, 2023 23:35
@jingyi-gao-ttd jingyi-gao-ttd restored the jyg-UID2-1736-show-correct-sharing-types branch October 17, 2023 06:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants