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

[285] Adjust Challenge Phase index query for Evaluators #359

Merged
merged 6 commits into from
Jan 15, 2025

Conversation

emmabjj
Copy link
Contributor

@emmabjj emmabjj commented Jan 15, 2025

Related ticket: #285

The list of challenges should only show the challenges that the evaluator has assigned submissions to.

Screenshot 2025-01-14 at 7 24 00 PM Screenshot 2025-01-14 at 7 24 23 PM

@emmabjj emmabjj self-assigned this Jan 15, 2025
@emmabjj emmabjj requested a review from stepchud January 15, 2025 01:27
)
end

it "only shows challenges with assigned submissions" do
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 can we add some specs for including status: :recused submissions and excluding status: :unassigned and status: :recused_unassiged?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@stepchud I added is_multi_phase: false to the spec to prevent the challenge factory from creating 3 phases for the challenge created. This is so the spec doesn't return challenge phases with no assigned submissions, which ends up looking like the behavior we're testing to avoid. 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

if the challenge is_muli_phase, and the evaluator is only assigned to one of the phases, we only want it to show that assigned phase in the list. probably the view should query for assigned/recused phases instead of challenges so that you can properly filter them? otherwise this isn't the correct behavior.

Copy link
Contributor

Choose a reason for hiding this comment

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

for example, here I have a multi-phase challenge and only assigned this evaluator to one of the phases, so they should only see the first and second row in this list, not the last two:

Screenshot 2025-01-15 at 8 59 33 AM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, that makes sense - I'll fix the query and update the tests, thanks!

@emmabjj emmabjj requested a review from stepchud January 15, 2025 02:59
@r-bartlett-gsa r-bartlett-gsa added this to the Sprint 01/14/25 milestone Jan 15, 2025
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.

needs to filter for assigned phases on is_multi_phase challenges

Comment on lines 114 to 117
expect(response.body).to have_css('td[data-label="Assigned to me"]')
expect(response.body).to have_css('td[data-label="Assigned to me"]', text: '2')
expect(response.body).to have_css('td[data-label="Remaining to evaluate"]')
expect(response.body).to have_css('td[data-label="Remaining to evaluate"]', text: '2')
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: these are duplicated expectations and can be collapsed to one line

Suggested change
expect(response.body).to have_css('td[data-label="Assigned to me"]')
expect(response.body).to have_css('td[data-label="Assigned to me"]', text: '2')
expect(response.body).to have_css('td[data-label="Remaining to evaluate"]')
expect(response.body).to have_css('td[data-label="Remaining to evaluate"]', text: '2')
expect(response.body).to have_css('td[data-label="Assigned to me"]', text: '2')
expect(response.body).to have_css('td[data-label="Remaining to evaluate"]', text: '2')

)
end

it "only shows challenges with assigned submissions" do
Copy link
Contributor

Choose a reason for hiding this comment

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

if the challenge is_muli_phase, and the evaluator is only assigned to one of the phases, we only want it to show that assigned phase in the list. probably the view should query for assigned/recused phases instead of challenges so that you can properly filter them? otherwise this isn't the correct behavior.

@emmabjj
Copy link
Contributor Author

emmabjj commented Jan 15, 2025

@stepchud I've adjusted the query so it queries on phases in which that evaluator has assigned or recused submissions instead of challenges.

The updated test now includes a multi-phase challenge. It checks that the evaluator only sees the phase of the multi-phase challenge in which they have assigned/recused submissions. And it still includes the expectations around including status: :recused submissions and excluding status: :unassigned and status: :recused_unassigned submissions.

Lemme know what you think! Thanks!

@emmabjj emmabjj requested a review from stepchud January 15, 2025 21:04
@emmabjj emmabjj merged commit d6f5fc1 into dev Jan 15, 2025
11 checks passed
@emmabjj emmabjj deleted the 285_list_of_challenge_scope_fix branch January 15, 2025 23:26
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