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

Badge system overhaul + Retired Logi #140

Merged
merged 25 commits into from
Aug 22, 2022

Conversation

samuelgrant
Copy link
Contributor

This PR resolves #135.

WARNING: This update requires you to run migration 0003!

  • The MySQL migration will move all badges from the admins table to the badge_assignment table. This migration is highly destructive, it is recommended that you back up the database before proceeding.

  • The Sqlite migration only creates the new tables and seeds the badge table. You will need to migrate badge_assignments manually.


This PR alters the way the waitlist handles specialist badges, the core changes are as follows:

  • Separated specialist badges (referred to as badges hereafter) from the ACL system
  • Allows an FC to retain specialist badges when they are granted FC permissions. This means they will not need to be reassigned badges if they leave the FC team
  • Created a new badges page where FCs can audit, assign and revoke badges
  • Removed the badge settings from the ACL modal which is now restricted to trainers/council/admin, and added a new badges modal
  • Added a Retired-Logi badge (defined in the migration)
  • Added logic in the invite controller to stop Trainees from inviting retired logi and Training Nestors (invite.rs#L69-L84)

Manage Badges

Users with shell access to the database can create, delete or update badges within the badge table.

SQL Example: Creating a badge
INSERT INTO badge (name) VALUES ('badge_name');

The badge name needs to match a KEY in the tagBadges object (see below) - otherwise, it will not render correctly
https://github.com/samuelgrant/tdf-waitlist/blob/79b0816875e2813b5d87bb865d12851081905da7/frontend/src/Components/Badge.js#L75-L87.

The badge order on the Xup card can be changed here: https://github.com/samuelgrant/tdf-waitlist/blob/79b0816875e2813b5d87bb865d12851081905da7/frontend/src/Pages/Waitlist/XCard.js#L29-L41


Change Log

  • Database:

    • Created table badge to manage tag options. These tags are used to dynamically populate UI elements on the badge page, and badge modals
    • Created table badge_assignments to track which badges have been assigned to a pilot, when and by whom
    • Provided SQL scripts for MySQL to migrate badges from the admins table into badge_assignments
  • Auth/Permissions

    • Removed the waitlist-tag scopes for badges
    • Removed the scopes that allow an FC to manage badges (e.g., access-manage:l) - The auth backend will be further developed in a later PR and legacy code will be removed
    • Created a new scope badges-manage used to access the badge page/modal and assigned it to HQ-FCs
    • Restricted the ACL modal to Trainers, Council and Admins
  • Controllers

    • Created a new routes file for badges
    • Added GET/POST/DELETE endpoints to manage badges
    • Updated the pilot info endpoint to include information about a pilot's badges
  • UI/UX

    • Created a badges page where badges can be assigned, revoked and audited
    • Removed the filters from ACL page - ACL management will be further improved in a future PR
    • Added a new NPM package react-data-table-component so e can render data tables on admin pages
    • Added the retired logi badge to the badge guide
  • Fleet

    • Created a Retired Logi badge that can be assigned by HQ-FCs,
    • Blocked trainees/advanced trainees from inviting retired logi and training nestors

Includes instructions for deploying a development build in WSL.

See #56 .
_todo_: Stop returning the details field \|\| remove the "ESI Error" prefix, return the entire error as JSON and let the endpoint/client work out how to handle it
* Removed the details from ESI Error messages
* Renamed ESIError::InviteFailed to ::WithMessage so it is more appropriate for use elsewhere
Migrations were tested in MySQL Workbench & sqlliteonline.com

Started the routes file. The other endpoints are written, but some debugging needs to be completed before I commit them.
- Installed a new Datatable library
- Setup a datatables component that uses the TDF theme and theme context
- Created the Badges Page and added a Delete button, basic sorting, and filtering

Todo:
- Create the Assign Badge modal
- Filter by badge type (e.g., Webs, Logi, etc)
- Clean up and improve code
- Fixed various bugs including some that would trigger console warnings
- Changed the background of the datatable from Accent1 to background
- Created a pilot search input (due to it's complexity it is in its own file)

todo:
- Code cleanup
- Setup the filter by badge type option

Flagged #135 to show progress
This is a big commit.
• Added a retired logi badge to the badge guide
• Added a red L-badge png (thanks HybridNZ)
• Completed the badges page
   • Completed the badge_type filter
   • Fixed undefined variable in state bug in the Assign Badge modal
   • Attached an htmlFor attribute to labels in the Assign Badge modal
   • Removed the WIP remark on the link to the badge page (FC Menu)
• Created an Assign Badges modal
  • Added modal to search page
  • Added modal to profile page
  • Added scope badges-manage
  • Removed badges tick boxes from ACL modal
  • Removed access to ACL modal from HQ FCs (must be >= trainer now)
  • Removed scopes to manage W, B, or L badges from the old system (old ACLs for badges is now readonly)
  • Tied announcements UI to the waitlist-tag:HQFC scope so that full FCs still have access
• Updated pilot profile endpoints to show ACL tag and badges. Before you could only have one or the other
• Added a RETIRED-LOGI badge preset

Plus other bug fixes, and code improvements.

Todo:
•  Make badges show up on X-UP
•  Run a check in the invite route to stop trainees from inviting non-L badged nestors (block invites to training & retired nestors)
•  Do load testing of the /fc/badges page to ensure it works properly with a large dataset
•  Work with Lia to create a system migration strategy
•  (?) Add revokved_by_id, revoked_at columns to BadgeAssignments for audit purposes (no plan to add a UI at this stage)

See #135
• Added new Retired-Logi badge
• Retired-Logi shows up behind LOGI
• Removed ACL Scopes for badges
Create badge_assignments based on rows in admins table, then delete non-fc rows from admins.
@testkil
Copy link
Contributor

testkil commented Aug 17, 2022

badges.rs is fucked

does not compile

@samuelgrant
Copy link
Contributor Author

Thanks for the heads up.

Compiling fine on my end, it could be a difference in our dev environment? I'm running Ubuntu.

Will fix this issue tomorrow.

@samuelgrant
Copy link
Contributor Author

badges.rs is fucked

does not compile

I can't repo these errors natively within my dev machine (Ubuntu, WSL2). I have been able to partially reproduce them within docker and I think I've fixed them, will commit the changes in a moment. However, I am unable to test the fix on my end due to some technical issues with Docker.

If you could pull the updated code, and attempt to build the app that would be fantastic.


Changes:
• Fixed five property capitalization issues in lines 128, 130-131 & 136-137
• Used the more traditional format!() method rather than the string interpolation method as this is unsupported by older versions of Rust.

@samuelgrant samuelgrant mentioned this pull request Aug 17, 2022
2 tasks
Change log:
- Improved the responsiveness of badge filters
- Made it so some badges cannot be assigned at the same time. This is done by adding an exclude rule in the badges table. When attempting to assign a badge the endpoint will check if the assignment would violate an exclude rule and if it would a BadRequest is returned
- Fixed a bug where the badge modal could cause an undefined error on the pilot profile page
- Updated the wording on the badges page - Code from @testkil
- Improved UI error handling for edge case errors
- Moved some of the base DataTable settings into the component so it can be reused
- Ran `cargo fmt`
- Ran `npx prettier write`
@samuelgrant
Copy link
Contributor Author

Okay, this PR should be ready for review @luna-duclos.

If you have been following along with the changes on your dev machine you will need to drop the badge and badge assignments table and re-run the relevant migrations as I have added a new column. I have checked all of the features that have been implemented to make sure they work as I would expect, but please take a good look as well before you merge.

Alex has brought a few UI concerns to me, after consideration and discussions with others I have opted not to implement his requests, my rationale is below:

  • Replace the badge filter select input with buttons.

    • When working with data tables a <select> element would be the typical component.
    • The new system can have as many badges as desired, this could get out of hand if there was a button for every filter. A select element provides a much cleaner UI element that scales nicely
  • Tie the Logi and Retired-Logi options into one filter

    • The system is dynamic, I am purposely trying to avoid hard-coding things so we can easily add/remove badges with as few code changes as possible. For this reason, the filters are dynamic, therefore the filters have to be separate
  • The order of badges on the badge modal isn't Alex's preferred order

    • Again dynamic system. It could be possible to sort alphabetically or to add a sort_index integer in the badge table. PRs on both these options would be easy in the future should it become necessary
  • One row per character per badge is too bloated. On the badges page show one row per character and create a history page to show who assigned what and when

    • Talked about this with other FCs who felt it was fine. Filters can mitigate the bloat concern
  • Data table design on the badges page is different to other admin pages (ACL/Bans)

    • I would like to update these pages over time to improve usability. They could be standardised at that time.

@samuelgrant samuelgrant marked this pull request as ready for review August 21, 2022 02:20
@luna-duclos luna-duclos merged commit 70c1f95 into the-ditanian-fleet:main Aug 22, 2022
luna-duclos added a commit that referenced this pull request Aug 22, 2022
luna-duclos added a commit that referenced this pull request Aug 22, 2022
luna-duclos pushed a commit that referenced this pull request Aug 30, 2022
* Added readme w/ dev deployment instructions.

Includes instructions for deploying a development build in WSL.

See #56 .

* Added contributing guidelines, fixed typos

* Fixed indentation issues.

* Fixed conflict in extensions.json

* Return  HTTP errors from ESI

_todo_: Stop returning the details field \|\| remove the "ESI Error" prefix, return the entire error as JSON and let the endpoint/client work out how to handle it

* Improve ESI error handling

* Removed the details from ESI Error messages
* Renamed ESIError::InviteFailed to ::WithMessage so it is more appropriate for use elsewhere

* Code cleanup

* Started on #135.

Migrations were tested in MySQL Workbench & sqlliteonline.com

Started the routes file. The other endpoints are written, but some debugging needs to be completed before I commit them.

* Finished endpoints, improved capitalisation consistency

* Created admin view

* Created Badges Page

- Installed a new Datatable library
- Setup a datatables component that uses the TDF theme and theme context
- Created the Badges Page and added a Delete button, basic sorting, and filtering

Todo:
- Create the Assign Badge modal
- Filter by badge type (e.g., Webs, Logi, etc)
- Clean up and improve code

* New Components + Badges Page

- Fixed various bugs including some that would trigger console warnings
- Changed the background of the datatable from Accent1 to background
- Created a pilot search input (due to it's complexity it is in its own file)

todo:
- Code cleanup
- Setup the filter by badge type option

Flagged #135 to show progress

* Badges UI, and ACL UI Cleanup

This is a big commit.
• Added a retired logi badge to the badge guide
• Added a red L-badge png (thanks HybridNZ)
• Completed the badges page
   • Completed the badge_type filter
   • Fixed undefined variable in state bug in the Assign Badge modal
   • Attached an htmlFor attribute to labels in the Assign Badge modal
   • Removed the WIP remark on the link to the badge page (FC Menu)
• Created an Assign Badges modal
  • Added modal to search page
  • Added modal to profile page
  • Added scope badges-manage
  • Removed badges tick boxes from ACL modal
  • Removed access to ACL modal from HQ FCs (must be >= trainer now)
  • Removed scopes to manage W, B, or L badges from the old system (old ACLs for badges is now readonly)
  • Tied announcements UI to the waitlist-tag:HQFC scope so that full FCs still have access
• Updated pilot profile endpoints to show ACL tag and badges. Before you could only have one or the other
• Added a RETIRED-LOGI badge preset

Plus other bug fixes, and code improvements.

Todo:
•  Make badges show up on X-UP
•  Run a check in the invite route to stop trainees from inviting non-L badged nestors (block invites to training & retired nestors)
•  Do load testing of the /fc/badges page to ensure it works properly with a large dataset
•  Work with Lia to create a system migration strategy
•  (?) Add revokved_by_id, revoked_at columns to BadgeAssignments for audit purposes (no plan to add a UI at this stage)

See #135

* Readd badges to xup card
• Added new Retired-Logi badge
• Retired-Logi shows up behind LOGI
• Removed ACL Scopes for badges

* Block trainees from inviting TNs and Retired Logi.

See #135

* Migrate badge assignments

Create badge_assignments based on rows in admins table, then delete non-fc rows from admins.

* Ignore eslint warning ==

* Ran npx prettier

* Fixed syntax issues in badges.rs

See #140 (comment) for details

* Bug Fixes & UI Improvements

* Ran npx prettier

* Minor UI/Backend tweaks & bug fixes...

Change log:
- Improved the responsiveness of badge filters
- Made it so some badges cannot be assigned at the same time. This is done by adding an exclude rule in the badges table. When attempting to assign a badge the endpoint will check if the assignment would violate an exclude rule and if it would a BadRequest is returned
- Fixed a bug where the badge modal could cause an undefined error on the pilot profile page
- Updated the wording on the badges page - Code from @testkil
- Improved UI error handling for edge case errors
- Moved some of the base DataTable settings into the component so it can be reused
- Ran `cargo fmt`
- Ran `npx prettier write`

* Fixed MySQL/SQLITE DB compatibility issues

* Bug Fixes
• Fixed a bug where a badge could not be assigned through the bulk assign modal if an excluded badge was being revoked after it. Now the modal will revoke badges before attempting to assign new badges
• Fixed an issue in the badge_exclude check's SQL query that meant a badge could not be assigned if an excluded badge had been assigned to ANY character. This was done by adding `AND characterId=?`
• Fixed a minor validation issue
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.

Inactive L badge
3 participants