-
Notifications
You must be signed in to change notification settings - Fork 226
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/users #1958
Merged
Merged
Lint views/users #1958
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lykimchee
requested review from
lykimchee
and removed request for
KesterTan
September 10, 2023 16:37
lykimchee
requested changes
Sep 10, 2023
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.
- Users list shows up
- Create new user form loads and works (except doesn’t add course, noted inside another comment)
- Editing user form works and loads
- User overview: delete user, change password, revoke token, new api activation, and managed authorized clients buttons all seem to work
lykimchee
approved these changes
Sep 12, 2023
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.
lgtm :>
NicholasMy
pushed a commit
to UB-CSE-IT/Autolab
that referenced
this pull request
Jan 5, 2024
* lint views/users * Address PR coments * add newline * fix test (cherry picked from commit ccd896b)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Summary generated by Reviewpad on 12 Sep 23 20:30 UTC
This pull request includes multiple file diffs with various code changes. Here is a summary of the changes:
The file
users.css.scss
added a new CSS class.users-list-header-container
, which sets the container's display to flex and aligns the content to the center with space between the items.The file
_form.html.erb
in theapp/views/users
directory has the following changes:<h2>
to<h3>
.class
attribute value has been changed to use hash syntax.The file
app/views/users/index.html.erb
has the following changes:<% content_for :stylesheets do %>
tag.<%= stylesheet_link_tag "users" %>
inside<% content_for :stylesheets do %>
tag.<div class="users-list-header-container">
.else
condition for when there are no users, displaying a message "There are currently no users that exist on Autolab.".The file
users/show.html.erb
has the following changes:The file diff for a file, which contains the form fields for the user model, includes fixing indentation and adding comments to the check_box field for administrators to provide help text.
The file
new.html.erb
has been modified to update the heading from<h1>New user</h1>
to<h2>Create New User</h2>
.The file
edit.html.erb
has the following changes:"_fields"
is now being passed thef
local variable.f.submit
button now has the class"btn primary"
.The file
users_controller_spec.rb
has the following changes:expect(response.body).to match(/New user/m)
has been modified toexpect(response.body).to match(/Create New User/m)
.expect(response.body).not_to match(/New user/m)
has been modified toexpect(response.body).not_to match(/Create New User/m)
.The file
lti_launch_initialize.html.erb
has been modified to add error handling when rendering the partial"home/topannounce"
. The code now uses abegin
andrescue
block to catch any exceptions that may occur during the rendering process and set the output tonil
if an error occurs.The file
users_controller.rb
has several changes, including updates to flash error messages and success messages in different actions.Please review these changes in the respective files for more details.
Description
erblint
onviews/users
usingbundle exec erblint app/users/announcements/*.html.erb -a
link_to raw('<span class="btn">Delete User</span>')
etc to have html inside ofdo
blockMotivation and Context
How Has This Been Tested?
Checklist:
overcommit --install && overcommit --sign
to use pre-commit hook for linting