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

[Misc] Admin panel unlink accounts #4198

Merged
merged 41 commits into from
Oct 23, 2024

Conversation

Opaque02
Copy link
Collaborator

@Opaque02 Opaque02 commented Sep 12, 2024

Why am I making these changes?

Walker wanted it
image
This has been requested because with the current admin linking panel, sometimes people would give us the wrong username to link to, so having the ability to unlink would be helpful. However, we also wanted to add the ability to find out when the last time a user played was for further verification reasons, so we've now added a new admin panel which shows the username, the linked discord/google (if they have one), the last time played and the date registered, allowing discord helpers to more easily verify users

What are the changes from a developer perspective?

A lot of stuff
  • Added code to the server that lets you find usernames, link/unlink discord by username, and returning an "admin result", which is what shows on the admin panel. This is currently made up of a username, discord id, google id, last logged on time and the date registered (seen below). The server code is needed for this to work, and can be found here
  • Changed the way form-modals are set up. This was because all the buttons/text/fields used to be done on setup(), but since the admin panel can have different buttons/text/fields, having it be a callable function works better. This required a slight update to the test dialogue panel to call things in the right place
  • For the admin panel option in the menu, you can now choose to open the linking panel directly or open the search panel, which will let you search a username.
    • If you're going to use the linking panel, the functionality and look are unaffected, but there's now a success message shown when you've successfully linked a username and discord. Similarly, if there's a username that doesn't exist in the system, an error message will show up letting you know the username doesn't exist. The linking panel now also keeps the username/discord filled in when showing an error/success message so you can edit things directly if need be, or see what you've linked/changed
    • When using the search panel, if the username exists in the server, the admin panel will load and automatically fill out the username, discord id, google id, last logged in date and registered date. The username, last logged in date and date registered will not be editable, and if the discord/google id exist on the server, they won't be editable either. If they don't exist, you'll be able to edit them to put in a discord/google id. The discord and google fields also have a button to link (if the discord/google don't exist on the server) or unlink (if they don't exist on the server). This allows the admin panel to be used to a) search to make sure a username exists, b) check and verify the details of a user from the database and c) link/unlink discord/google ids of a user from the server. The last thing is the admin panel has a button to go back to the search panel so you can do multiple users at once
  • On the login page, the gear that users open to see their username now shows an error code (PO1 or PO2) for no saved files or too many saved files on record. These error codes have been hard-coded into the game rather than being part of the translation repo to make it easier going forward. This makes it easier for discord helpers to see what the error message is showing if a user's game is in a different language the helpers don't know
  • Added a new abstract function to the form-modal-ui-handler (getInputFieldConfigs(args: any)) that allows you create an array of a new interface (InputFieldConfigs) that lets you have specific info about a field in a form modal. It currently has support for the label, whether it's a password field, and whether the field is read only or not. Due to this, everything that extends form-modal-ui-handler (the admin panel, renaming panel, login form, registration form and test dialogue panel) have had their own version of getInputFieldConfigs() implemented
Things to note

There's a weird ui issue still happening that we can't figure out why it's happening. When using the admin panel, it will always have text to return - sometimes it's the username/discord id (in the case of the linking panel), and sometimes it's the admin info details (username, discord, google, last logged in date and registered date). For some reason, when there's text being returned that are from input text fields, there's 3-4 frames during the loading part of the admin panel where the text appears in the top left corner of the screen, before disappearing and showing up in the correct space on the admin panel. We couldn't figure out why this is happening, but it's minor and still works, and since such a small number of people (non-players) see this panel anyway I thought it'd be ok to leave as is, though I'm open to discussion about it to try and fix it. Walker said it should be fine, but if we happen to figure out why it's happening and how to fix it, I can update things.

Screenshots/Videos

Please note that for these screenshots/videos, the discord/google ids used are fake and won't actually work. They are just used to show updates to the DB rather than actually logging in with google/discord

Login panel updates:

No save file (PO1) error

image

Too many save files (PO2) error

image

For these errors, the translation team has been made aware that this is now too long for the panel and they are in the process of updating things

No error messages

image

Test Dialogue:

Here is a video of me opening the test dialogue for testing
2024-10-14.09-18-35.mp4

Admin panel stuff:

The Linking Panel (this functionality should be unchanged):

DB before

image

Video of changes (notice the new success message and the fact that the username and discord are now shown after linking so you can see what you've done)
2024-10-14.09-46-12.mp4
DB after

image

Here's screenshot of me filling out the linking panel with a username that isn't in the database

image

Since the username/discord ID are kept after hitting link, here's an example of using the linking panel with just a username and no discord ID - you can see that the error still says that a discord ID is needed, but the username is already filled for you so you can use it again

image

Similarly, if we just have a discord ID and no username, you get the same error but about username missing instead

image

Username Search/Admin Panel:

Here's a video of the process of going through a username search and opening the admin panel
2024-10-14.09-56-12.mp4
As you can see, this user already has a discord and google ID linked to their username, so both items were filled. We can also see the last date played and date registered.

Because this user already has a discord and google ID, we have the option to unlink those from the DB by pressing the icon to the right of the discord/google fields.

In the below example, we can see a different user who only has their discord linked, and which we will unlink (and see the success message), and then we can link a google account to their user (and see the success message for that):

Also worth noting is that for the discord ID (and the google one once linked), because that comes from the server, it is a readonly value and can't be edited. This is to prevent accidental changes. To change a user's discord/google ID, you would need to unlink it first, then enter the new details and then link it again.

Here's the DB before

image

And after

image

Admin panel error codes:

Here's what happens when you don't have a username in the search bar for the admin panel

image

Here's what happens when you try to link a discord/google ID without filling in the field

image

image

How to test the changes?

This first part is copied from the server PR so you can reference how to get the server working properly

You need to set up your own server and pull this PR. Any questions with that, ask me in discord, or ask Walker, though we have this thread in discord that we used to get things set up - it's a lot of info but it should have a lot of troubleshooting steps that we did to get it working.

Once your server is set up, make sure you also have access to the DB. I'm using MySQLWorkbench to let me see the db, but you can use anything as long as it can connect to a maria DB instance.

Once everything is set up, in your server, you'll need to change the some files. If you would like to see any of these in action, you can check some previous commits to see what I used to have for my testing, since I committed my testing changes:

First there's a file called beta.env in the main project folder that looks like this:

VITE_BYPASS_LOGIN=0
VITE_BYPASS_TUTORIAL=0
VITE_SERVER_URL=http://{serverIP}:8001
VITE_DISCORD_CLIENT_ID=1248062921129459756
VITE_GOOGLE_CLIENT_ID=955345393540-2k6lfftf0fdnb0krqmpthjnqavfvvf73.apps.googleusercontent.com
VITE_I18N_DEBUG=1
debug=true
dbaddr=db
dbuser=pokerogue
dbpass=pokerogue
dbname=pokeroguedb 
gameurl=http://{devMachineIP}:8000 
callbackurl=http://{serverIP}:8001

Here I've used {serverIP} in a few places and {devMachineIP} for the gameurl - for me, my server machine and my dev machine are two different machines, so my gameurl is a different IP compared to my server IP. However, if this is not the case, you should be able to use your dev machine's IP for all cases of devMachineIP and serverIP instead.

Next, in your docker-compose.Example.yml, under services->server add the following code:

env_file:
      - beta.env
    image: rogueserver:latest

This makes the game use your beta.env file from above, and also makes sure to pull the edited version of rogueserver instead of master branch. Finally, add the following to services->db:

ports:
      - "3306:3306"

This makes sure the DB is using the right port.

To run it, open a cmd at the location of your rogueserver folder and run this code:

docker build ./ -t rogueserver

This will build the server using the code provided. Once that's done, use this code to set everything up using the docker-compose file we updated before:

docker-compose -f docker-compose.Example.yml up -d

Hopefully if you go to run it, all 3 parts will be good, and going into the server to see the logs will show you the daily seed (see below):

image
image

This part is new and not in the rogueserver PR

Once that's all done, you can now go to the pokerogue repo and get that up and running by changing a couple of files. Firstly, change your .env.development file by changing the first line (VITE_BYPASS_LOGIN=1) to be VITE_BYPASS_LOGIN=0 instead, and change line 3 (VITE_SERVER_URL=http://localhost:8001) to have your server url from above. For example, I used VITE_SERVER_URL=http://192.168.1.101:8001 since my server was running at 192.168.1.101

Next in the utils.ts file, lines 285, 290, 293 and 294 all have cookie related stuff for them. Within each of those lines is "Secure;" - you want to remove it from all of the cookie related lines - this means you'll be changing this:

  document.cookie = `${cName}=${cValue};Secure;SameSite=Strict;Domain=${window.location.hostname};Path=/;Expires=${expiration.toUTCString()}`;

To this:

  document.cookie = `${cName}=${cValue};SameSite=Strict;Domain=${window.location.hostname};Path=/;Expires=${expiration.toUTCString()}`;

Make sure to remove the ; when removing "Secure" so you don't end up with double ;

Now that everything's done, you should be able to start the game up. Once the game is up, you should be able to register an account and log in. Once that's done, log out and make a few accounts - this will let you test things much easier for this PR.

Once that's done, you'll need an admin discord account. I made an account with my pokerogue name (Opaquer) and linked that to my discord ID, which I then used throughout testing. You'll need to link your discord ID to your server account (manually or otherwise). To do that through the DB, you can use something like (make sure to remove the {} when entering values):

`update pokeroguedb.accounts 
set pokeroguedb.accounts.discordId = '{discordID}'

where pokeroguedb.accounts.username = '{username}'

Note that the discordId should be an 18 char string of numbers in the real world, but you can use anything for testing. Your username should be whatever username you want your admin account to be for the server. As an example, this is what my code looked like:

`update pokeroguedb.accounts 
set pokeroguedb.accounts.discordId = '256000469158068224'

where pokeroguedb.accounts.username = 'Opaquer'

Back in the server code, /api/account/discord.go change this function to immediately return true for your discord ID that's now linked to your server admin account. I did it by doing this:

func IsUserDiscordAdmin(discordId string, discordGuildID string) (bool, error) {
	return discordId == "{discordID}", nil

Where the discordID should be what you used before. In my case, that second line would be return discordId == "256000469158068224", nil. Again make sure to remove the {} when entering values

Now that this is all done, you should be good to test everything! Here's the best way to test various things
  • The Admin discord ID only lets the appropriate account(s) see the admin panel: when logged into your admin account, open the menu, go to community and you should see an "Admin" option. Log out of your account and log into one of your other accounts that don't have an admin discord linked and you won't see that admin option
  • Admin panel - Linking tool: Go to the admin menu option and choose the link option. You can enter in a username in the database and use a random string of numbers for the discord ID. When done, it should say it worked and have the success message, and updating the DB should show that too. You can also use username that aren't in the DB to check the error handling, and not entering a username or discord ID
  • Admin panel - Search/Admin panel: Go to the admin menu and choose search. Enter a username in the DB and it'll open the admin panel for that user. Check to make sure everything looks right from the data, and also to make sure that linking/unlinking work as expected. You can also check to make sure that if a field is filled out from the DB, it shouldn't be editable at all. Once that's all good, you can test to make sure the error handling works by searching for usernames that aren't in the system, or for leaving out required fields, such as the username from the search panel, or the discord/google ID when trying to link them
  • Test dialogue: open the test dialogue from menu->manage data->test dialogue and see if it opens/works. Here is the original PR if you want a quick refresher on how it works

Checklist

  • I'm using beta as my base branch
  • There is no overlap with another PR?
  • The PR is self-contained and cannot be split into smaller PRs?
  • Have I provided a clear explanation of the changes?
  • Have I considered writing automated tests for the issue?
  • If I have text, did I make it translatable and add a key in the English locale file(s)?
  • Have I tested the changes (manually)?
    • Are all unit tests still passing? (npm run test)
  • Are the changes visual?
    • Have I provided screenshots/videos of the changes?

@flx-sta flx-sta added Enhancement New feature or request Miscellaneous Changes that don't fit under any other label and removed Miscellaneous Changes that don't fit under any other label labels Sep 13, 2024
@Opaque02 Opaque02 changed the title [QoL] Admin panel unlink accounts [Beta][QoL] Admin panel unlink accounts Sep 25, 2024
@Opaque02 Opaque02 requested a review from a team as a code owner October 15, 2024 06:05
@MokaStitcher
Copy link
Contributor

PR with code cleanup suggestions: Opaque02#3

Copy link
Contributor

@MokaStitcher MokaStitcher left a comment

Choose a reason for hiding this comment

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

lgtm
Mostly doc + scope change suggestions
The most important doc is in FormModalUiHandler since it'll help future devs who want to extend the class understand what they need to do. These and more generally doc for anything that is not private should use /** */ style comment so that the IDE picks up on it (within reason of course, we don't want to comment every single thing if they are self explanatory)

Not asking to change it but in general I think it would be better if the inline comments were on their own line unless they are really short. Some of these are too long to show in a single line so it makes things harder to read

Copy link
Contributor

@MokaStitcher MokaStitcher left a comment

Choose a reason for hiding this comment

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

Might as well so it's not forgotten

Full discussion in discord UI thread, but we also may want a specific error message if a discord/google id is already linked to another account

Copy link
Contributor

@MokaStitcher MokaStitcher left a comment

Choose a reason for hiding this comment

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

sorry, meant to request changes 💀

MokaStitcher
MokaStitcher previously approved these changes Oct 20, 2024
Copy link
Contributor

@MokaStitcher MokaStitcher left a comment

Choose a reason for hiding this comment

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

ship it 👀

@MokaStitcher
Copy link
Contributor

For these errors, the translation team has been made aware that this is now too long for the panel and they are in the process of updating things

oh I didn't even noticed that earlier. For reference the various error messages were already too long in some languages before adding those error codes. And I'm pretty sure I've seen a PR before to handle that by resizing the text if needed or something? But I can't find it so who knows lol

@DayKev DayKev changed the title [Beta][QoL] Admin panel unlink accounts [Misc] Admin panel unlink accounts Oct 22, 2024
@DayKev DayKev added Miscellaneous Changes that don't fit under any other label Blocker This PR blocks other PRs and should be prioritized labels Oct 22, 2024
flx-sta
flx-sta previously approved these changes Oct 23, 2024
@innerthunder innerthunder dismissed stale reviews from flx-sta and MokaStitcher via a341b52 October 23, 2024 19:53
@MokaStitcher MokaStitcher merged commit a2419c4 into pagefaultgames:beta Oct 23, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocker This PR blocks other PRs and should be prioritized Enhancement New feature or request Miscellaneous Changes that don't fit under any other label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants