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

[209] Submission Stats #280

Merged
merged 17 commits into from
Dec 11, 2024
Merged

[209] Submission Stats #280

merged 17 commits into from
Dec 11, 2024

Conversation

stonefilipczak
Copy link
Contributor

@stonefilipczak stonefilipczak commented Nov 20, 2024

#209
Adds a stats component to the submissions list page for challenge managers.

Screen Shot 2024-11-19 at 10 10 23 PM

@stepchud stepchud force-pushed the 209_submission_stats branch from 68d7200 to cfeabcc Compare November 20, 2024 15:43
@stepchud stepchud marked this pull request as draft November 20, 2024 23:16
@stepchud stepchud added the WIP label Nov 20, 2024
@stepchud
Copy link
Contributor

This will be updated with status from the Evaluation records, which are currently in review on #281

Copy link
Contributor

@stepchud stepchud left a comment

Choose a reason for hiding this comment

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

First review pass

app/views/phases/_submissions_stats.html.erb Outdated Show resolved Hide resolved
app/views/phases/_submissions_stats.html.erb Outdated Show resolved Hide resolved
app/views/phases/submissions.html.erb Outdated Show resolved Hide resolved
app/views/phases/_submissions_stats.html.erb Outdated Show resolved Hide resolved
@stepchud stepchud force-pushed the 209_submission_stats branch from cfeabcc to e5c9c0f Compare November 22, 2024 23:25
@stepchud stepchud removed the WIP label Nov 26, 2024
@stepchud stepchud marked this pull request as ready for review November 26, 2024 00:44
Copy link
Contributor

@stepchud stepchud left a comment

Choose a reason for hiding this comment

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

(I think) I made the necessary code changes as far as the stats component calculations and display. However, I would like to see some tests for this component. They should mostly be request specs in spec/requests/submissions_spec.rb that assert the various stats are displayed correctly. Can we also add a spec/system/submissions_spec.rb expectation that the stats show visible content on the page.

stonefilipczak and others added 2 commits December 11, 2024 09:04
Co-authored-by: Stephen Chudleigh <stepchud@users.noreply.github.com>
Comment on lines 58 to 59
submission = create(:submission, challenge: challenge, phase: phase)
submission_2 = create(:submission, challenge: challenge, phase: phase, judging_status: "selected")
Copy link
Contributor

Choose a reason for hiding this comment

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

Lint/UselessAssignment

Suggested change
submission = create(:submission, challenge: challenge, phase: phase)
submission_2 = create(:submission, challenge: challenge, phase: phase, judging_status: "selected")
create(:submission, challenge: challenge, phase: phase)
create(:submission, challenge: challenge, phase: phase, judging_status: "selected")

get submissions_phase_path(phase)
expect(response.body).to include("Boston Tea Party Cleanup")
# total submissions
expect(response.body).to include("2")
Copy link
Contributor

Choose a reason for hiding this comment

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

this is not testing much since the expect below also has a 2 in it, but I found a way to use css selectors that gives more specific expectations. the trick is to include Capybara::RSpecMatchers in the request specs to enable the have_css() matcher. I'll add this now.

Copy link
Contributor

@stepchud stepchud left a comment

Choose a reason for hiding this comment

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

nice work, I added the rspec matcher update e20849c

@stepchud stepchud merged commit 1223827 into dev Dec 11, 2024
11 checks passed
@stepchud stepchud deleted the 209_submission_stats branch December 11, 2024 17:50
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