Skip to content

Studio: Allow blocking user from adding demo sites - #354

Merged
katinthehatsite merged 20 commits into
trunkfrom
add/ui-for-when-user-is-blocked
Jul 26, 2024
Merged

Studio: Allow blocking user from adding demo sites #354
katinthehatsite merged 20 commits into
trunkfrom
add/ui-for-when-user-is-blocked

Conversation

@katinthehatsite

@katinthehatsite katinthehatsite commented Jul 11, 2024

Copy link
Copy Markdown
Contributor

Related to https://github.com/Automattic/dotcom-forge/issues/7370

Proposed Changes

This PR implements the UI for when the user is blocked from creating a demo site:

Screenshot 2024-07-11 at 9 10 57 PM

Testing Instructions

  • Navigate to the User Report Card for your own user
  • Use the Disable site creation button (at the bottom of the User Report Card) to block your user from creating demo sites
  • Pull the changes from this branch
  • Start the app with nvm use && npm install && npm start
  • Make sure to log in with the user that you just blocked from creating sites
  • Navigate to Share tab
  • Confirm that Add demo site button is disabled
  • Hover over the button and confirm that it displays a message about demo sites being disabled for this account

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?
@katinthehatsite katinthehatsite self-assigned this Jul 11, 2024
Comment thread src/hooks/use-archive-error-messages.ts
@katinthehatsite
katinthehatsite requested a review from a team July 11, 2024 19:17
@wojtekn

wojtekn commented Jul 12, 2024

Copy link
Copy Markdown
Contributor

@katinthehatsite could we also account for a case in which we disable demo sites for the user who has Studio already open? Currently, when such a user tries to add a demo site, they will get a proper error message, but the button won't be disabled:

Screenshot 2024-07-12 at 12 22 03

@katinthehatsite

Copy link
Copy Markdown
Contributor Author

Hmm let me check what is happening there 👀

@katinthehatsite

Copy link
Copy Markdown
Contributor Author

@wojtekn I made some updates, would you be able to give it another try?

Comment thread src/hooks/use-snapshots.tsx Outdated
Comment thread src/components/content-tab-snapshots.tsx Outdated

@fluiddot fluiddot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I noticed that if the user being blocked already has demo sites created, the Update demo site button is enabled. I'd apply the same approach we've done for the Add button there.

Screenshot 2024-07-18 at 10 56 51

Additionally, when updating a demo site, I think the error message for blocked users should mention the failure reason

Screenshot 2024-07-18 at 10 37 17
Comment thread src/components/content-tab-snapshots.tsx Outdated
Comment thread src/hooks/use-snapshots.tsx
Comment thread src/components/content-tab-snapshots.tsx Outdated
Comment thread src/components/content-tab-snapshots.tsx Outdated
@katinthehatsite

Copy link
Copy Markdown
Contributor Author

Thanks for the review @fluiddot ! I made some changes to block the Update site button when the user is blocked in 0b95433

I tried the alternative solution but I think at the end, it comes to the same thing that @wojtekn mentioned initially with the app not updating the data in real-time. I left it as is for now, let's wait for some comments and see if we should change the approach of polling the endpoint.

@katinthehatsite
katinthehatsite requested review from a team and sejas July 25, 2024 13:03

@sejas sejas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I've created a PR to simplify the logic of the update demo site button. Let me know what do you think. #388

I've tested login in, loging out, enabling and disabling my user RC setting, and it works as expected. Great job!

Uwi625.mp4

From my point of view I would avoid adding the active listening. I see it as a corner case.
I like how we refresh the state when the user starts the app or logs in. We should use a similar criteria as we do with the quota of demo sites.

Instead of adding an active listening, I would disable the button after the first fail. Not a strong opinion.

Comment thread src/components/content-tab-snapshots.tsx Outdated
Comment thread src/components/content-tab-snapshots.tsx Outdated
Comment thread src/components/content-tab-snapshots.tsx
Comment thread src/hooks/use-snapshots.tsx
Comment thread src/components/content-tab-snapshots.tsx
@katinthehatsite

Copy link
Copy Markdown
Contributor Author

From my point of view I would avoid adding the active listening. I see it as a corner case.
I like how we refresh the state when the user starts the app or logs in. We should use a similar criteria as we do with the quota of demo sites.

Based on our discussions of this issue, I have implemented the behaviour mentioned in the comment above. Since it is more of an edge case, I would be okay not to poll the endpoint and update the information in real-time.

@sejas could you please give it another test and let me know what you think? Thanks!

@katinthehatsite
katinthehatsite requested a review from sejas July 25, 2024 19:30

@sejas sejas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Kat!, thanks for applying the suggested cahnges.

I've tested the edge case when a user gets blocked and it has Studio running.
As shown in the video, the create demo site will fail and immediately will see the tooltip 👍 .

zo12GS.mp4
@katinthehatsite
katinthehatsite merged commit 96a2b46 into trunk Jul 26, 2024
@katinthehatsite
katinthehatsite deleted the add/ui-for-when-user-is-blocked branch July 26, 2024 10:55
@fluiddot

Copy link
Copy Markdown
Contributor

Thanks for the review @fluiddot ! I made some changes to block the Update site button when the user is blocked in 0b95433

Thanks @katinthehatsite for addressing the comments I shared. I've tested the changes in trunk and confirmed it worked as expected 🎊 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants