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

lint views/course_user_data #1964

Merged
merged 2 commits into from
Sep 12, 2023
Merged

Conversation

20wildmanj
Copy link
Contributor

@20wildmanj 20wildmanj commented Aug 20, 2023

Summary

Summary generated by Reviewpad on 20 Aug 23 03:11 UTC

This pull request includes the following changes:

  • In the file show.html.erb, the formatting of a line was updated to include the raw method in the tweak method. This ensures correct display in HTML.

  • In the file edit.html.erb, changes were made to update the title variable, modify the form_for method, and render a partial with updated local variables. These changes enhance the user experience and improve form functionality.

  • In the file user.html.erb, the entire content was deleted. The file previously contained HTML code for displaying user information and the ability to edit it.

  • In the file sudo.html.erb, changes include formatting revision, updated paragraph tag, and the addition of the class: "btn primary" attribute to the form tag. These changes aim to improve the appearance and functionality of the sudo feature.

  • The file app/views/course_user_data/index.html.erb was deleted. It previously contained HTML code for displaying account details for a user.

  • In another file, changes include indentation adjustments, addition and removal of help texts, modification of text fields, checkboxes, and labels. These changes enhance the form's usability and readability.

  • In the file new.html.erb, changes involve modifying the URL in an AJAX request, removing a comment, updating the @title variable, reformatting the locals hash, and updating the onclick attribute. These changes primarily focus on code formatting and minor functional adjustments.

Description

  • Lint files within views/course_user_data using bundle exec erblint app/views/course_user_data/*.html.erb -a
  • Remove index.html.erb and user.html.erb, as it seems that neither is used
    • For index.html.erb, the index action already uses show, couldn't find any references. Also, the erb file references @user.andrewID which shouldn't really exist anymore, and this file hasn't really been touched
    • For user.html.erb, similar story, couldn't find any references, looks the same as show.html.erb, only recent change was with @damianhxy 's PR Add mononym support #1580 adding Mononym support
  • Some visual touchups for _fields.html.erb so that the "Tweak" container no longer has left padding

Motivation and Context

  • As part of summer linting

How Has This Been Tested?

  • Verify there are no usages of index.html.erb and user.html.erb
  • Ensure that viewing and editing user's CUDs works (Go to "Manage Course" -> "Manage Users" -> Click on a User), for both instructor and student (student should only be able to change nickname, not see tweak info etc.)
  • Ensure that "Enroll User" still works and autofill works for existing users
  • Verify that "Act as User" button still works (in "Manage Course")

Checklist:

  • I have run rubocop for style check. If you haven't, run overcommit --install && overcommit --sign to use pre-commit hook for linting

@reviewpad reviewpad bot added medium Pull request is medium waiting-for-review labels Aug 20, 2023
@damianhxy
Copy link
Member

damianhxy commented Sep 10, 2023

There are some other outdated references to andrewId, e.g. _autoCompleteAndrewID.html.erb, autograderDev.html.erb, and download_roster.html.erb. Maybe those files can be removed too?

EDIT: I realize you removed download_roster.html.erb in another PR

Copy link
Member

@damianhxy damianhxy left a comment

Choose a reason for hiding this comment

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

File changes look safe

  • index.html.erb and user.html.erb unused
  • viewing and editing CUDs works for both instructors and students
  • "Enroll user" still works, autofill works for existing users
  • act as user still works

I think some other dead files can be removed, but otherwise LGTM

@20wildmanj
Copy link
Contributor Author

Other dead files will be removed in other PRs

@20wildmanj 20wildmanj added this pull request to the merge queue Sep 12, 2023
Merged via the queue into master with commit 26c693a Sep 12, 2023
6 checks passed
@20wildmanj 20wildmanj deleted the joeywildman-lint-course-user-data branch September 12, 2023 16:11
NicholasMy pushed a commit to UB-CSE-IT/Autolab that referenced this pull request Jan 4, 2024
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.

2 participants