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

Dev to Main #12

Merged
merged 52 commits into from
Nov 30, 2024
Merged

Dev to Main #12

merged 52 commits into from
Nov 30, 2024

Conversation

invaderb
Copy link
Member

feat: add rating endpoints
feat: add scheduled job
test: add tests for rating generation
chore: add endpoints to readme
chore: code clean up and refactor

invaderb and others added 30 commits May 5, 2023 19:23
feat: add scheduled job
test: add tests for rating generation
chore: ad endpoints to readme
fix: add missing api param
fix: make optional columns
fix: add missing db names to typeorm feature and root
feat: add scheduled job
test: add tests for rating generation
chore: ad endpoints to readme
fix: add missing api param
fix: make optional columns
fix: add missing db names to typeorm feature and root
feat: add scheduled job
test: add tests for rating generation
chore: ad endpoints to readme
chore: add better api property descriptions
invaderb added 13 commits June 18, 2024 11:19
chore: refactor to make variables more meaningful
chore: refactor ratings to have better typings
chore: refactor dto files
chore: update formatting
chore: fix typos
chore: fix typings
chore: clean up player dto
chore: update readme
…en't changed

chore: add additional comments to explain the generateRatings
feat: rating list to  to json file
@invaderb invaderb requested review from gruppler, nitzel and humanat June 21, 2024 17:58
@invaderb invaderb self-assigned this Jun 21, 2024
Copy link
Collaborator

@nitzel nitzel left a comment

Choose a reason for hiding this comment

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

Awesome, I like the .bru tests!

Also crazy job you got the rating script running here! I'm still confused why fatigue can be string/Fatigue/null though ^^

It looks like the rating is neither running as a Cron nor does an endpoint exist to trigger it manually, so how are the ratings going to be calculated on production?

.github/workflows/deploy-dev.yaml Show resolved Hide resolved
previous.txt Show resolved Hide resolved
src/models/dto/events/events.dto.ts Outdated Show resolved Hide resolved
src/models/dto/players/player.dto.ts Outdated Show resolved Hide resolved
participation_rating: number;

@ApiProperty()
fatigue: Fatigue | string | null;
Copy link
Collaborator

Choose a reason for hiding this comment

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

What does it mean if it's a string/null?

Copy link
Member Author

Choose a reason for hiding this comment

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

New registered players Fatigue is null until the rating script runs and adds it but when it gets parsed to json that where the Fatigue type comes in

Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this be fatigue?: Fatigue then?

Copy link
Member Author

Choose a reason for hiding this comment

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

@nitzel man it's a good thing I'm looking through this again from your comments I found a issue with the fatigue logic after adjusting this.

src/models/ratings/ratings.service.spec.ts Outdated Show resolved Hide resolved
src/models/ratings/tasks/rating.task.ts Outdated Show resolved Hide resolved
src/models/ratings/ratings.service.ts Outdated Show resolved Hide resolved
@invaderb
Copy link
Member Author

@nitzel fatigue gets saved as a string since its a json object but it can be null until a new player gets rated

Since the cron time gets pulled from the environment file you cant use the @Cron directive its using the cron schedule registry in the module init.

chore: code clean up and refactor
fix: update fatigue to return player
fix: player type class
fix: update fatigue unit test and types
test: add unit test for parsing the fatigue
@invaderb invaderb merged commit ea18424 into main Nov 30, 2024
2 checks passed
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.

3 participants