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

feat: Used frontend-toolkit in galactic-sovereign-frontend #240

Conversation

Knoblauchpilze
Copy link
Owner

@Knoblauchpilze Knoblauchpilze commented Jan 4, 2025

Work

Building on what we did in #226 for the user-dashboard, this PR uses the new types and elements defined in the frontend-toolkit in the galactic-sovereign-frontend project. The goal is similar to what we achieved in the user-dashboard: make sure that we have a consistent way to access and interpret the information coming from our API. Additionally, it is a good opportunity to improve a bit the structure of the project by tidying up the logic in the $lib folder.

What we did in this PR focuses on:

  • clearly separating the DTOs from the business logic.
  • unified error handling, using what is provided by the frontend-toolkit as a base.
  • use of converters to aggregate the data coming from the backend and what we need for the UI (see e.g. buildingActionConverter.

Tests

General testing

As for the user-dashboard project we don't have tests of any kind for this project. The testing was done manually by inspecting all the pages of the website.

The improved error handling can be seen for example when the user fails to login due to invalid credentials. This is how it looks like in production before this PR:

image

And this is how it looks like now:

image

Wrong interpretation of the level of a building

During this process, we noticed that we wrongly interpret the level of buildings and this leads to inconsistencies in the buildings view. Take this example from the production website:

image

With a fresh account without any building, the displayed production for the metal mine is 33 with a gain of 3 for level 1. This is wrong: the first level is producing 30, which is a gain of 30 compared to not having a metal mine at all. We fixed it in this PR and it now shows as follows:

image

As shown, with a level 1 mine we now correctly indicate that the next level (so level 2) will produce 33 metal which represents a gain of 3 compared to the current production. The crystal mine is at level 0 and shows a gain of 20 with the level 1: this is consistent with what we expect.

Future work

As mentioned in #226, we already started to extract the various services from this repository into their own dedicated repositories. This is essentially breaking down the monorepo we had to move towards a micro-service architecture. This needs to be worked on in another PR.

Update:

Comment on lines +50 to +52
return fail(HttpStatus.UNPROCESSABLE_ENTITY, {
message: 'Please select an action'
});
Copy link
Owner Author

Choose a reason for hiding this comment

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

Used the recommended approach to handle validation errors (see documentation). The old approach was not really relevant anymore as none of the success or missing properties were used. It's already arguable if the message is useful here as the 'form' we have to create a building action is not reacting to it.

@@ -13,7 +13,7 @@
"format": "prettier --write ."
},
"dependencies": {
"@totocorpsoftwareinc/frontend-toolkit": "^0.0.18"
"@totocorpsoftwareinc/frontend-toolkit": "^0.0.22"
Copy link
Owner Author

Choose a reason for hiding this comment

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

Used the latest version of the frontend-toolkit project.


export const backToLobby = async ({ cookies }: RequestEvent) => {
resetGameCookies(cookies);
redirect(303, '/lobby');
redirect(HttpStatus.SEE_OTHER, '/lobby');
Copy link
Owner Author

Choose a reason for hiding this comment

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

Replaced the raw HTTP codes with constants coming from the frontend-toolkit.

readonly key: string = '00000000-0000-0000-0000-000000000000';
readonly validUntil: Date = new Date();

constructor(data: object) {
Copy link
Owner Author

Choose a reason for hiding this comment

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

This is an example of how we extracted DTOs from the implementation file. It is much easier to know what is where than before. Additionally we also correctly provided default values that are meaningful compared to what is returned by the backend.

Comment on lines -9 to +12
building: UiBuilding;
availableResources: UiResource[];
building: PlanetBuildingUiDto;
availableResources: PlanetResourceUiDto[];
Copy link
Owner Author

Choose a reason for hiding this comment

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

The new types are just drop-in replacements from the old ones. This is by design.

Comment on lines +5 to +8
export function buildingActionResponseDtoToBuildingActionUiDto(
apiDto: BuildingActionResponseDto,
buildingDto: BuildingResponseDto | undefined
): BuildingActionUiDto {
Copy link
Owner Author

Choose a reason for hiding this comment

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

The logic to convert from the XYResponseDto (coming from the API) to the XYUiDto (used in the frontend) is now extracted in a separate package: this helps with code structure.

@@ -1,83 +0,0 @@
import { error, redirect } from '@sveltejs/kit';

export class ResponseEnvelope {
Copy link
Owner Author

Choose a reason for hiding this comment

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

This is provided by the frontend-toolkit package.

@@ -0,0 +1,3 @@
export function computeCostForLevel(cost: number, progress: number, level: number): number {
Copy link
Owner Author

Choose a reason for hiding this comment

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

The game package is now composed of only the game logic: the actions and interaction with the API are moved to dedicated packages.

Comment on lines -21 to +29
const universesResponse = await getUniverses(sessionCookies.apiKey);
analayzeResponseEnvelopAndRedirectIfNeeded(universesResponse);
const universes = responseToUniverseArray(universesResponse);
let apiResponse = await getUniverses(sessionCookies.apiKey);
redirectToLoginIfNeeded(apiResponse);
handleApiError(apiResponse);

const universes = parseApiResponseAsArray(apiResponse, UniverseResponseDto);
if (universes.length === 0) {
error(HttpStatus.INTERNAL_SERVER_ERROR, 'Failed to get server data');
}
Copy link
Owner Author

Choose a reason for hiding this comment

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

Restructured the +page.server.ts files to use the new logic: it is slightly cleaner but more importantly it is also using the same approach as provided in the user-dashboard. This is nice to validate that the logic works essentially the same way: we can now more confidently try to extract the core logic into the template-frontend repository.

@Knoblauchpilze Knoblauchpilze merged commit 648edd2 into master Jan 4, 2025
11 checks passed
@Knoblauchpilze Knoblauchpilze deleted the feature/use-frontend-toolkit-in-galactic-sovereign-frontend branch January 4, 2025 15:37
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.

1 participant