Feature/auth rework #44
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Add more validation on art/design credit linking
Thanks to Cy for table structure - Create user alias table - Add has_alias bool to users table - Alter User model to suit, create UserAlias model - Add basic url to sites config for construction of links from aliases - Add command for moving existing aliases from users table to user_aliases table
- Index both the above and url column of character image creators - Consolidate alias update commands into one and update character image creator command for new system
- Update character logs - Update alias migration command with character logs - Add helper function to check if an alias belongs to a site used for auth and if so, to a user - Add helper function to just return 'name'@'site' without link - Tidy up art/design credits
…d time - Fix issues with case sensitivity in alias comparisons
corowne
requested changes
Nov 21, 2020
There was a problem hiding this comment.
Looks good, just a few things:
- Admin panel (editing a user) - in the clear alias section, the alias textbox still uses the old alias as its value so the box is blank
- Admin\Users\UserController.php - related to the above; postUserAlias (to clear the old alias) needs to be updated to use the new alias table too
- BrowseController.php - masterlist character/MYO/sublist search by username still searches on owner_alias, user search by alias still searches on alias column on users
(Also not sure if within scope of this update - items still use artist_alias)
- Add item artist alias migration to command - Add more feedback to alias migration command as it runs - Add validation to character owner URL - Add tag stripping to report URLs - Make submission URL nullable/non-required
…iases - Update user alias clearing for new storage system/to support multiple aliases - Fix display bug on admin index page
|
All requested changes made-- I updated item artists to the new system as well. Going to take a look at addressing #49 before requesting review again though. |
corowne
requested changes
Nov 28, 2020
…text for multiple aliases - Add checks to see if user has remaning aliases, and if any are valid primary aliases, in alias clear processing
itinerare
added
the
reviewed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
--drop-columns, which drops the alias columns when it is finished moving information either to user_id or url columns. It's recommended to run it without once and ensure data has been moved properly before running it again with the option.