Add a CCE Minor Note feature attached to the Notes section in the User profile - #1737
Add a CCE Minor Note feature attached to the Notes section in the User profile#1737munsakad wants to merge 24 commits into
Conversation
|
Overall Impression The feature makes sense and the approach of mirroring Bonner tag behavior is a smart, consistent design decision. The template references row.isCCEMinorNote and submits a cceMinor form field, but there are no backend changes in this
|
MImran2002
left a comment
There was a problem hiding this comment.
These are the changes or the bugs that I encounter:
Fix 1: I think this issue alone should not be solved with a HTML. This is broader because this could potentially include javascript and python. (Hint: look at how bonner notes work)
Fix 2: Because the code only holds html I am not seeing any cce minor flags from my side:
(These three notes are created by toggling cceminor)
Fix 3: When I toggle the CCEminor and save the note, just like the bonner if i were to create a bonner note and go back and look i will see the model has bonner note toggle
bonner example:
cceminor:
if its cceminor the toggle should be toggled to show it is working.
Fix 4: In an instance where both the bonner and cce minor are toggle the bonner notes accordion seems to overwrite the cceminor notes and the note only appear in bonner this scenario should be considered as in should the note that have both toggles need to appear on both or one of them.
|
@munsakad and @brightfietsop-ux after the department issue you guys worked on, while waiting for that issue to be reviewed and merged. Worked on this as we don't want this to be dragged into the weeks ahead. |
What i discovered before coding
SummaryThis PR adds support for marking profile notes as CCE Minor notes alongside the existing Bonner note option. Changes
TestingTested the following note combinations:
Verified in MySQL that:
|
…My Proposals to not change to Manage Proposals when on an account that wasn't the displayed student.
…t failure on github
| if not noteTextbox: | ||
| return "Note cannot be empty", 400 | ||
|
|
||
| addProfileNote( |
There was a problem hiding this comment.
This does not handle a failed note insert. For example, if the user doesn't exist (e.g., postData["username"] isn't a real user), this function fails, and the route fails. Handle the failure using try: except:.
The logic function should raise and the route function should try/except.
| profileNoteID = request.form.get("id") | ||
| noteTextbox = request.form.get("noteTextbox", "").strip() | ||
| visibility = request.form.get("visibility", 1) | ||
| bonner = request.form.get("bonner") == "yes" | ||
| cceMinor = request.form.get("cceMinor") == "yes" |
There was a problem hiding this comment.
Both this and the one above can be a single function that reforms the request data. use (optional?) input parameters to handle the slight differences.
| visibility, | ||
| bonner, | ||
| cceMinor, | ||
| noteTextbox, | ||
| username, | ||
| ): |
There was a problem hiding this comment.
make a single line since it is a function definition (not a dictionary or some other datatype)
| mysql -u root -proot -NBe "SELECT COUNT(*) FROM information_schema.COLUMNS WHERE TABLE_SCHEMA='celts' AND TABLE_NAME='profilenote' AND COLUMN_NAME='isCCEMinorNote';" | | ||
| grep -q '^1$' || | ||
| mysql -u root -proot celts -e "ALTER TABLE profilenote ADD COLUMN isCCEMinorNote TINYINT(1) NOT NULL DEFAULT 0 AFTER isBonnerNote;" |
There was a problem hiding this comment.
Remove this from the PR, should not be here. For local testing only .
|
Also your is only for the test data if you want to use the production data before after ./database/run_database.sh from-backup , you should log into mysql and enter ALTER TABLE profilenote |
Issue Description
Fixes issue #1405
Changes
-Display a "CCE Minor" prefix in the note's Visible To column, mirroring the existing Bonner prefix convention.
-Pass the flag through the Edit button via a data-cceminor attribute so the modal can pre-fill it.
-All changes are confined to app/templates/main/userProfile.html.
Testing
Open a profile for a user with CCE Minor interest and click Add Note, confirm that the "Related to CCE Minor" toggle appears.
For a user without CCE Minor interest, confirm the toggle is hidden.
Verify a note flagged as CCE Minor shows the "CCE Minor" prefix in the Visible To column.
Confirm that the Bonner toggle and existing note behavior are unchanged.