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

Disable user data fields during CUD edit #1562

Merged
merged 3 commits into from
Jul 17, 2022
Merged

Conversation

damianhxy
Copy link
Member

Description

  • Disable :email, :first_name and :last_name fields on edit CUD page, but enable on new CUD page

Motivation and Context

In #877, a hotfix was applied to address a bug which prevented users from adding students to a course. However, the modified partial is also used by the edit CUD page. This means that when editing CUDs, the email, first name, and last name of students can be changed. As commented by @cg2v, it doesn't make sense for the fields to be edited here (first name and last name can be edited via manage users, email shouldn't be edited at all).

This PR adds a local parameter when calling the fields partial to determine whether the above mentioned fields should be disabled.

How Has This Been Tested?

  • When adding user to course, email, first name and last name fields should be editable
  • When editing course user, email, first name and last name fields should not be editable

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

@damianhxy damianhxy changed the title Disable necessary fields during CUD edit Disable user data fields during CUD edit Jul 16, 2022
Copy link
Contributor

@20wildmanj 20wildmanj left a comment

Choose a reason for hiding this comment

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

  • Verified editing first and last name disabled on CUD works properly

Just have a question now, I guess this means instructors wouldn't be able to change first/last names of students (since I assume instructors usually aren't given admin perms), so I wonder if instructors care about not having that ability?

@damianhxy
Copy link
Member Author

Just have a question now, I guess this means instructors wouldn't be able to change first/last names of students (since I assume instructors usually aren't given admin perms), so I wonder if instructors care about not having that ability?

I think it should not matter. After all, when instructors add students, the name fields are auto populated after you enter the email, so it's not really something they should be editing.

Screen Shot 2022-07-17 at 16 10 20

Besides, the name is something that students themselves can edit, so there should be no need for instructors to be involved in the process.

@20wildmanj
Copy link
Contributor

Just have a question now, I guess this means instructors wouldn't be able to change first/last names of students (since I assume instructors usually aren't given admin perms), so I wonder if instructors care about not having that ability?

I think it should not matter. After all, when instructors add students, the name fields are auto populated after you enter the email, so it's not really something they should be editing.

Screen Shot 2022-07-17 at 16 10 20

Besides, the name is something that students themselves can edit, so there should be no need for instructors to be involved in the process.

Just have a question now, I guess this means instructors wouldn't be able to change first/last names of students (since I assume instructors usually aren't given admin perms), so I wonder if instructors care about not having that ability?

I think it should not matter. After all, when instructors add students, the name fields are auto populated after you enter the email, so it's not really something they should be editing.

Screen Shot 2022-07-17 at 16 10 20

Besides, the name is something that students themselves can edit, so there should be no need for instructors to be involved in the process.

Alright that makes sense, sounds good to me!

@damianhxy damianhxy merged commit f8f5da4 into master Jul 17, 2022
@damianhxy damianhxy deleted the disable-cud-fields branch July 17, 2022 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants