ucsb-cs156-s21 / proj-ucsb-cs-las

https://proj-ucsb-cs-las.herokuapp.com/
MIT License
2 stars 3 forks source link

6pm-1 CX/KW -user couldn't edit tutor name when they update office hour #287

Closed PureDreamerGk closed 3 years ago

PureDreamerGk commented 3 years ago

In this PR, we update the office hour form so that the user will not be able to edit the tutor when they update the office hour.

PureDreamerGk commented 3 years ago

We need to add a test on TutorDisplay component

github-actions[bot] commented 3 years ago

Nice PR! Your PR can be merged once you get an approving review from a staff member and a member of your team.

Keep in mind, code reviews only count if you use the "Approve" or "Request changes" option in GitHub. You can find this under the "Files changed" tab under the "Review changes" button.

PR review process

Generated by :no_entry_sign: dangerJS against 86fb2e2c49fcfc7b9057e0c250eef2646d7e4bad

codecov[bot] commented 3 years ago

Codecov Report

Merging #287 (86fb2e2) into main (33fda2d) will increase coverage by 0.00%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##               main     #287   +/-   ##
=========================================
  Coverage     99.84%   99.84%           
  Complexity      405      405           
=========================================
  Files            89       90    +1     
  Lines          1927     1928    +1     
  Branches         93       93           
=========================================
+ Hits           1924     1925    +1     
  Misses            3        3           
Impacted Files Coverage Δ
.../src/main/components/OfficeHours/OfficeHourForm.js 100.00% <ø> (ø)
...mponents/TutorAssignment/TutorAssignmentDisplay.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 33fda2d...86fb2e2. Read the comment docs.

kenny-wang6 commented 3 years ago

I believe for this issue, we want you to remove the editable part of the form pertaining to the tutor altogether on the edit page, and only use the TutorAssignmentSelector when creating a new assignment. In this current state of this branch, it looks like the selector still exists, and the tutor's name is displayed in text below it.: image

We only want the text displayed, so that when editing an office hour we cannot edit the tutor assigned to that hour.

My bad, I ended up fixing the tests but didn't realize the implementation had bugs after changing the code. Will work on fixing both issues now.

pconrad commented 3 years ago

This is good for 10 points when merged.