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

[Refactor] Pokerogue API client #4583

Merged
merged 59 commits into from
Nov 4, 2024

Conversation

flx-sta
Copy link
Collaborator

@flx-sta flx-sta commented Oct 4, 2024

What are the changes the user will see?

n/a

Why am I making these changes?

Well you know how it is... the way it was designed in the code was bugging @Opaque02 & me. I thought I fast and easy fix it. One thing leads to another and here we are..

Warning

I couldn't check the discord/google unlink requests due to local limitations.

Yes, I did set up my own Discord Dev Application to link it to but it simply refused to save those discord IDs into the database. No idea why
image

What are the changes from a developer perspective?

All Utils.apiFetch and Utils.apiPost were replaced with a new PokerogueApi class which is structured into sub-apis

Every request & response from the API is properly typed now

Screenshots/Videos

n/a

How to test the changes?

  1. Make sure you have the rogueserver repo cloned
  2. Start the local server
  3. Make sure to set VITE_BYPASS_LOGIN to 0 in env.development
  4. Start the client
  5. Tests all the things that cause requests

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? => there is some already
  • Have I tested the changes (manually)?
    • Are all unit tests still passing? (npm run test)
  • [] Are the changes visual?

@flx-sta flx-sta added the Refactor Rewriting existing code related label Oct 4, 2024
@flx-sta flx-sta marked this pull request as ready for review October 4, 2024 23:58
@flx-sta flx-sta requested a review from a team as a code owner October 4, 2024 23:58
@flx-sta flx-sta changed the title [Refactor] Pokerogue API [Refactor] Pokerogue API client Oct 5, 2024
@DayKev DayKev requested a review from Opaque02 October 30, 2024 20:38
@PigeonBar
Copy link
Collaborator

These are now unused after the refactor, should we delete them or mark them as @deprecated?

pokerogue/src/utils.ts

Lines 273 to 276 in 684fb93

export const localServerUrl = import.meta.env.VITE_SERVER_URL ?? `http://${window.location.hostname}:${window.location.port + 1}`;
// Set the server URL based on whether it's local or not
export const apiUrl = localServerUrl ?? "https://api.pokerogue.net";

@flx-sta
Copy link
Collaborator Author

flx-sta commented Nov 1, 2024

These are now unused after the refactor, should we delete them or mark them as @deprecated?

pokerogue/src/utils.ts

Lines 273 to 276 in 684fb93

export const localServerUrl = import.meta.env.VITE_SERVER_URL ?? `http://${window.location.hostname}:${window.location.port + 1}`;
// Set the server URL based on whether it's local or not
export const apiUrl = localServerUrl ?? "https://api.pokerogue.net";

@deprecated sounds good as a first step

src/system/game-data.ts Outdated Show resolved Hide resolved
@PigeonBar
Copy link
Collaborator

The Discord and Google unlink requests should be partially testable by linking using the admin panel, then unlinking using the admin panel or by using Menu > Manage Data > Unlink.

@PigeonBar
Copy link
Collaborator

I've done some testing and haven't found any issues so far

@flx-sta
Copy link
Collaborator Author

flx-sta commented Nov 4, 2024

@PigeonBar Applied all your suggested changes.
Thanks

@Tempo-anon Tempo-anon merged commit 7a0c88e into pagefaultgames:beta Nov 4, 2024
13 checks passed
@flx-sta flx-sta deleted the refactor/api-requests branch November 4, 2024 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Refactor Rewriting existing code related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants