Skip to content

S08 judging#103

Merged
stefdworschak merged 27 commits intoCode-Institute-Community:masterfrom
Sarosim:S08-Judging
Oct 29, 2020
Merged

S08 judging#103
stefdworschak merged 27 commits intoCode-Institute-Community:masterfrom
Sarosim:S08-Judging

Conversation

@Sarosim
Copy link
Contributor

@Sarosim Sarosim commented Oct 25, 2020

Description

This PR includes the rendering of the judging page according to the ‘Judging Page’ wireframe, as well as handling the score submission and saving it to the model.

Pull request type

S08 As Staff, I would like to Judge each submission.

Related Issue

none existing, one question to be decided:
The Project Score Categories model doesn't have description field for the score categories, so the wireframes couldn't be fully followed. I have included screenshots with and without descriptions, if we want to have the descriptions, the model needs to be adjusted.

Configuration instructions

needs to have Hackathon event with finish date in the past, participating team in the event, valid user as judge for the event, project assigned to the team and scores to be non existent for the hackathon and the team by the judge. Also, project score categories have to be present in the respective model.

Testing

Four verifications are included:

  • whether user is judge for the event
  • if event is ready to be judged (finished)
  • whether the selected team belongs to the selected event
  • if the judge has already scored the requested team's Project (Issue #S08. As Staff, I would like to Judge each submission. #37 says: “Cannot edit scores once submitted”)
    all four tested.

Scores cannot be submitted without valid score for each and every project score categories
Responsiveness tested in Chrome Dev Tools screen shots included

Screenshots

image

image

image

Additional Information

none

Does this introduce a breaking change

  • Yes
  • No

@Sarosim
Copy link
Contributor Author

Sarosim commented Oct 26, 2020

Pulled the latest PRs from upstream, updated this branch and fixed merge conflicts.

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 the PR looks good. There are a few comments for you. I think the main thing to look at is using an extra judging status in the hackathon instead of the end_date to check if judging should be allowed.

Also, can you create a follow-up PR to create the page to list all team projects for a hackathon and link from the hackthon to the list of all projects page and then to the scoring page?

user_is_judge = True
if not user_is_judge:
messages.error(request, "You are not a judge for that event!")
template = 'home/index.html'
Copy link
Member

Choose a reason for hiding this comment

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

This seems to be defined a couple of times which is not needed, I would just add the template directly to the render function without declaring it as a variable.

Same for all instances below.

return render(request, template)

# verify if event is ready to be judged (finished)
finish = event.values('end_date')[0]['end_date']
Copy link
Member

Choose a reason for hiding this comment

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

Not sure this is the right trigger. If the event is finished, it is over and done with I would think. Maybe instead of that we can add another status to the hackathon called judging and add that.


# verify that the selected team belongs to the selected event
team_belongs_to_event = False
for team in event.values('teams'):
Copy link
Member

Choose a reason for hiding this comment

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

Again this could probably be simplified.

return render(request, template)

# check if the judge has already scored the requested team's Project
the_event = get_object_or_404(Hackathon, pk=hack_id)
Copy link
Member

Choose a reason for hiding this comment

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

You already have this defined further up, so I would reuse that instance.

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

Sarosim commented Oct 28, 2020

reviewed all the change requests and solved them, have not made migrations but pushed all other changes.

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 LGTM

Please just fix the two small comments and I will merge this.

@stefdworschak stefdworschak added the hacktoberfest-accepted Accepted PR during Hacktoberfest label Oct 28, 2020
@Sarosim
Copy link
Contributor Author

Sarosim commented Oct 29, 2020

All done @stefdworschak

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 not sure if I overlooked some of these things, but I think you introduced some new chnages here. Could you please make the few additional changes before merging? Thanks

@Sarosim
Copy link
Contributor Author

Sarosim commented Oct 29, 2020

all done @stefdworschak

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 merged commit b867689 into Code-Institute-Community:master Oct 29, 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.

#S08. As Staff, I would like to Judge each submission.

2 participants

Comments