-
Notifications
You must be signed in to change notification settings - Fork 2
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
[83] Edit Evaluator's Score #422
Conversation
…evaluators-score * 'dev' of github.com:GSA/Challenge_platform: [84] Submission Advancement (#397)
…evaluators-score * 'dev' of github.com:GSA/Challenge_platform: 57 | Move submission evaluation status specs [266] Header Updates (#414) 57 | Have service update the model directly to set evaluation_status codeclimate 57 | Move logic to calculate the evaluation status of a submission into a service 57 | Refine approach to update the submission's evaluation_status 57 | code climate 57 | Update and track evaluation_status through submission, evaluation submission assignment, and evaluation for filtering 57 | Migration - add evaluation_status to submission table
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still reviewing, I'll go ahead and leave comments as they come up rather than wait for the full review to submit feedback all at once.
app/models/user.rb
Outdated
def full_name(format: :default) | ||
return "Unknown User" if first_name.blank? && last_name.blank? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure where this function came from but can we display the email address if the evaluator does not have any name associated? It shouldn't happen often because we require and update their first_name and last_name when they are added to the panel. :last_first
format doesn't seem to be used rn.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just made something since we didn't have any consistent full name functionality for users/evaluators. But I can just do what is done in the other places. Mostly wanted to make sure potential white space, missing values, or anything else was handled in the same way
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's nothing wrong with adding the function, but I don't think it should say "Unknown User" when there is a known user, they just don't have a first_name or last_name. we have a bug (which we should try to fix) in the invite flow where it's not always saving the name of the user after they accept the invite and I'd prefer not to highlight the missing name here. instead we have the email or it could use the username part of the email before the @
symbol.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the function is currently being used to show the name to the challenge manager and they already have access to the user's information on the evaluation panel so it's not leaking data.
There is an issue with the "Revised Score" total. If the input is empty, or there's no radio button selected, the "Revised Score" should show up in the UI as the original evaluator score. Just the display value. The input should remain null by default until the CM enters a different score, so it doesn't override the evaluator's score with the same value and they can update just the scores they want to modify and don't have to input values for all. Screen.Recording.2025-02-18.at.10.03.38.PM.mov |
…evaluators-score * 'dev' of github.com:GSA/Challenge_platform: 58 | Add submitter email to Submissions csv file (#428)
…latform into 83/edit-evaluators-score * '83/edit-evaluators-score' of github.com:GSA/Challenge_platform: Update app/controllers/evaluations_controller.rb
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚢 ⚓
#83