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

Add highest_score field to HackProjectScoreCategory model #81

Merged

Conversation

Sarosim
Copy link
Contributor

@Sarosim Sarosim commented Oct 18, 2020

S08 - Judging submissions
Score Categories can have different score ranges (e.g. 1-10, 1-15)
Adding this field sets the upper end of the scale, so the admin can define for each scoring category the scale by providing the highest possible score for that given category.

I'm just realising that this field was indeed on the original schema (with name score), but wasn't implemented in the models. In this respect it can be treated as a fix.

This might affect other user stories so I'm submitting this PR without any other changes to S08, so others can pull it and start working with this fix.

Tests:
Added the field to the test case but no assertion is added for it. I run the tests and all pass.

Know bugs/errors:
None

In my version, the field is named highest_score, but I can rename it, if you guys deem necessary.

image

Copy link
Member

@stefdworschak stefdworschak left a comment

Choose a reason for hiding this comment

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

@Sarosim some minor changes, but I would also add an assertion to test this is working.

@@ -166,6 +166,9 @@ class HackProjectScoreCategory(models.Model):
on_delete=models.CASCADE,
related_name="hackprojectscorecategories")
category = models.CharField(default="", max_length=255)
# Score Categories can have different score range (e.g. 1-10, 1-15)
# this field sets the upper end of the scale
highest_score = models.IntegerField(default=10)
Copy link
Member

Choose a reason for hiding this comment

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

I'd say change it to max_score which is probably more fitting. Highest score sounds more like something related to a highscore to me.

Copy link
Contributor Author

@Sarosim Sarosim Oct 18, 2020

Choose a reason for hiding this comment

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

Thanks Stefan, obviously min_score and max_score are the most meaningful names, corrected, tested with assertions, all tests passing.

@TravelTimN TravelTimN linked an issue Oct 18, 2020 that may be closed by this pull request
@Sarosim
Copy link
Contributor Author

Sarosim commented Oct 18, 2020

Added min_score field with default=1 and renamed highest_score to max_score, added assertions for both fields to the tests and run them. All tests passing

Copy link
Member

@stefdworschak stefdworschak left a comment

Choose a reason for hiding this comment

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

LGTM

@stefdworschak stefdworschak added the hacktoberfest-accepted Accepted PR during Hacktoberfest label Oct 18, 2020
@stefdworschak stefdworschak merged commit e82ad5e into Code-Institute-Community:master Oct 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted Accepted PR during Hacktoberfest
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants