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

Sort scoreboard beyond third column, improve CUD validations #1925

Merged
merged 14 commits into from
Jun 21, 2023

Conversation

damianhxy
Copy link
Member

@damianhxy damianhxy commented Jun 15, 2023

Summary

Summary generated by Reviewpad on 21 Jun 23 18:44 UTC

This pull request includes changes to several files. The file 'scoreboards.md' has a minor change, sorting all columns in descending order and tie-breaking by submission time. The view file 'app/views/course_user_data/new.html.erb' has several updates, including adding a javascript_include_tag and modifying the onclick function of the submit button. The Scoreboard view has also been updated in the 'scoreboard.rb' file in the 'app/models' directory, including a validation check for the column specification and refactored code to handle all scoreboard columns. Changes have been made to the CourseUserDataController in the Rails application, including modifying error messages and allowing for HTML in the flash[:error] message. The form validation function and nickname validation method have also been updated.

Description

  • Refresh scoreboard#show page
  • Scoreboard now sorts all columns, not just up to the third column
  • Update docs to reflect change
  • Fix CUD form validation
  • Remove unnecessary error display code: they basically don't work because errors (during create / update) are cleared during redirects, so this PR uses flashes instead. A user would not be able to load the scoreboard if they had errors in their CUD because they would have been redirected to the edit page.

Motivation and Context

Currently, only the first three columns of any scoreboard is sorted (when using a custom column specification). This PR remedies that so all columns are now sorted.

Note: I don't expect this to break any scoreboard code, since scoreboards generally consist of numeric data. Even if they contain string data, to_f casts non-numeric strings to 0.0, which is as good as not sorting.

Closes #970

Furthermore, the custom form validation for the nickname fields is currently broken and does not work. Validation errors are also not shown since a redirect takes place.

This PR fixes the validation code, shows errors in flashes instead, and adds more visual cues (e.g. set maxlength = 32, indicate in help text)

How Has This Been Tested?

Install the following assessment (after unzipping):
randomlab_20230616.tar.zip

Submit any file using three different student accounts. View scoreboard.

Before

Specification: {"scoreboard": [ {"hdr":"Problem 1"}, {"hdr":"Problem 2"}, {"hdr":"Problem 3"}, {"hdr":"Problem 4"}, {"hdr":"Problem 5"}, {"hdr":"Problem 6"} ] }
Observe: PROBLEM 6 (no asc or desc)
Screenshot 2023-06-16 at 15 16 20

Observe: last column not sorted (tiebreak by time)
Screenshot 2023-06-16 at 15 16 28

After (Descending)

Specification: {"scoreboard": [ {"hdr":"Problem 1"}, {"hdr":"Problem 2"}, {"hdr":"Problem 3"}, {"hdr":"Problem 4"}, {"hdr":"Problem 5"}, {"hdr":"Problem 6"} ] }
Observe: PROBLEM 6 [desc]
Screenshot 2023-06-16 at 15 14 11

Observe: last column sorted in descending order
Screenshot 2023-06-16 at 16 27 52

After (Ascending)

Specification: {"scoreboard": [ {"hdr":"Problem 1"}, {"hdr":"Problem 2"}, {"hdr":"Problem 3"}, {"hdr":"Problem 4"}, {"hdr":"Problem 5"}, {"hdr":"Problem 6", "asc": 1} ] }
Observe: PROBLEM 6 [asc]
Screenshot 2023-06-16 at 15 14 43

Observe: last column sorted in ascending order
Screenshot 2023-06-16 at 16 28 11

Others

  • Check documentation

Column specification

  • Try saving a specification of true -> get error Colspec must be a hash object
  • Try saving a specification which is missing quotes around one of the strings -> get error (similar to) Colspec 451: unexpected token at '{hdr:"Problem 6"} ] }'

Form validation

  • Test form validation on scoreboard page, edit CUD, new CUD pages: attempt to save nickname with non-ascii characters (e.g. 你好) -> observe alert, and highlighting + focusing of nickname field
Screenshot 2023-06-16 at 16 24 11 Screenshot 2023-06-16 at 16 24 14
  • Observe helptext additionally states "(maxlength: 32)"
Screenshot 2023-06-16 at 16 28 35 Screenshot 2023-06-16 at 16 28 49
  • You can also disable frontend validation and try saving a nickname of length > 32 and with non-ascii characters (e.g. 12345678123456781234567812345678你好)

Updating via Scoreboard or Edit page

Screenshot 2023-06-16 at 16 35 07

Creation

Screenshot 2023-06-22 at 02 43 08

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have run rubocop for style check. If you haven't, run overcommit --install && overcommit --sign to use pre-commit hook for linting
  • My change requires a change to the documentation, which is located at Autolab Docs
  • I have updated the documentation accordingly, included in this PR

@reviewpad reviewpad bot added the small Pull request is small label Jun 15, 2023
@reviewpad reviewpad bot added medium Pull request is medium and removed small Pull request is small labels Jun 15, 2023
@reviewpad reviewpad bot added small Pull request is small and removed medium Pull request is medium labels Jun 15, 2023
@reviewpad reviewpad bot added medium Pull request is medium and removed small Pull request is small labels Jun 16, 2023
@damianhxy damianhxy changed the title Add scoreboard sort beyond third column Sort scoreboard beyond third column, improve CUD validations Jun 16, 2023
@damianhxy damianhxy marked this pull request as ready for review June 16, 2023 08:37
@reviewpad reviewpad bot requested a review from najclark June 16, 2023 08:37
@damianhxy damianhxy marked this pull request as draft June 17, 2023 02:18
@damianhxy damianhxy marked this pull request as ready for review June 17, 2023 17:53
Copy link
Contributor

@najclark najclark left a comment

Choose a reason for hiding this comment

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

lgtm, testing behavior looks correct

app/views/course_user_data/_fields.html.erb Outdated Show resolved Hide resolved
@damianhxy damianhxy enabled auto-merge June 21, 2023 18:44
@damianhxy damianhxy added this pull request to the merge queue Jun 21, 2023
Merged via the queue into master with commit e62ca22 Jun 21, 2023
6 checks passed
@damianhxy damianhxy deleted the scoreboard-sort branch June 21, 2023 18:49
NicholasMy pushed a commit to UB-CSE-IT/Autolab that referenced this pull request Jan 3, 2024
…#1925)

* Update styling

* Allow columns beyond col 3 to be sorted

* Update documentation

* Remove unnecessary checks

* Update colspec validations

* Remove unnecessary error display

* Add maxlength 32 to nickname field

* Fix syntax of valid_nickname? validation

* Display errors for edit page directly in flash

* Skip validation if nickname is nil

* Standardize new page with edit page

* Update validation js

* Add space in maxlength

* Make error message consistent

(cherry picked from commit e62ca22)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
medium Pull request is medium
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Scoreboard Column Specification does not work if asc/desc is defined beyond third column
2 participants