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

Display suspension to user #41

Merged
merged 21 commits into from Dec 12, 2021
Merged

Display suspension to user #41

merged 21 commits into from Dec 12, 2021

Conversation

imorland
Copy link
Member

Addresses flarum/framework#1447

Recreation of #34, before that got polluted

  • Expand SuspendUserModal to include two new optional fields: reason (for moderator use) and message (for display to the suspended user
  • Display a modal to a suspended user, displaying the message entered by the moderator, and the length of the suspension
  • Move Form-group components to an ItemList for extensibility
  • compat exports
  • Expand notification blueprints to include email notifications of suspension/unsuspension, including the suspension message

image

image

image

Copy link
Sponsor Member

@askvortsov1 askvortsov1 left a comment

Choose a reason for hiding this comment

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

Thank you for this, will be very useful! In addition to the above comments, I think the translations/wording could be a bit clearer as to the difference between "reason" and "message"

js/src/forum/components/SuspendUserModal.js Outdated Show resolved Hide resolved
js/src/forum/components/SuspensionInfoModal.js Outdated Show resolved Hide resolved
js/src/forum/components/SuspensionInfoModal.js Outdated Show resolved Hide resolved
js/src/forum/components/SuspensionInfoModal.js Outdated Show resolved Hide resolved
js/src/forum/components/SuspensionInfoModal.js Outdated Show resolved Hide resolved
src/AddUserSuspendAttributes.php Show resolved Hide resolved
src/AddUserSuspendAttributes.php Outdated Show resolved Hide resolved
src/Listener/SaveSuspensionToDatabase.php Outdated Show resolved Hide resolved
Copy link
Sponsor Member

@askvortsov1 askvortsov1 left a comment

Choose a reason for hiding this comment

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

Had the opportunity to try it out locally, a few thoughts:

  • Indefinite suspensions aren't shown as such to the user, neither in the alert modal or the notification. It just says "16 years".
  • Is the suspend reason supposed to be accessible to admins anywhere?

locale/en.yml Outdated Show resolved Hide resolved
@askvortsov1
Copy link
Sponsor Member

Also, would the associated migrations pose issues for existing communities with tens of millions of users?

@imorland
Copy link
Member Author

* Is the suspend reason supposed to be accessible to admins anywhere?

Yes, in the suspend modal:

image

image

@imorland
Copy link
Member Author

Also, would the associated migrations pose issues for existing communities with tens of millions of users?

My understanding after looking at this exact issue for a previous employer is that when adding a new column to existing tables, defaulting to null is the most performant. As we are setting nullable => true, then this will be the case here

@imorland
Copy link
Member Author

imorland commented Nov 16, 2021

* Indefinite suspensions aren't shown as such to the user, neither in the alert modal or the notification. It just says "16 years".

Yes, indefinite is displayed in the alert modal:

image

@imorland
Copy link
Member Author

imorland commented Nov 16, 2021

However the alert notification does need some work, as indefinite displays the remaining time until 2038 :( - I'll get on this ASAP

image

@askvortsov1
Copy link
Sponsor Member

* Indefinite suspensions aren't shown as such to the user, neither in the alert modal or the notification. It just says "16 years".

Yes, indefinite is displayed in the alert modal:

image

Hmm I didn't see that when I tested. I'll try again

@askvortsov1
Copy link
Sponsor Member

@imorland https://i.imgur.com/D4RQfS1.png possibly a timezone issue?

@imorland
Copy link
Member Author

@imorland https://i.imgur.com/D4RQfS1.png possibly a timezone issue?

Yes, very much so..

@imorland
Copy link
Member Author

imorland commented Dec 2, 2021

@imorland https://i.imgur.com/D4RQfS1.png possibly a timezone issue?

This should now be resolved 🤞

js/src/forum/components/SuspendUserModal.js Show resolved Hide resolved
js/src/forum/checkForSuspension.ts Outdated Show resolved Hide resolved
@imorland
Copy link
Member Author

@SychO9 sorry about that, I think I've got it as you requested now 🤞

Copy link
Member

@SychO9 SychO9 left a comment

Choose a reason for hiding this comment

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

Epic! thank you very much!

@SychO9 SychO9 merged commit 4ac6887 into flarum:master Dec 12, 2021
askvortsov1 pushed a commit that referenced this pull request Mar 11, 2022
askvortsov1 pushed a commit that referenced this pull request May 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants