Skip to content

[Bugfix:Forum] Grader upduck message correction#11358

Merged
bmcutler merged 3 commits into
Submitty:mainfrom
sujay-d07:grader-upduck-issue
Feb 12, 2025
Merged

[Bugfix:Forum] Grader upduck message correction#11358
bmcutler merged 3 commits into
Submitty:mainfrom
sujay-d07:grader-upduck-issue

Conversation

@sujay-d07

@sujay-d07 sujay-d07 commented Jan 24, 2025

Copy link
Copy Markdown
Contributor

Please check if the PR fulfills these requirements:

  • Tests for the changes have been added/updated (if possible)
  • Documentation has been updated/added if relevant
  • Screenshots are attached to Github PR if visual/UI changes were made

What is the current behavior?

Closes #11355

When the Grader upducked any thread or reply, there is the "liked by teaching staff" message but when the page is refreshed the message gets hidden as the accessFullGrading() was getting called, this prevented the message to be displayed when a Grader upducked the post/reply.

Screencast.from.24-01-25.01.15.14.PM.IST.webm

What is the new behavior?

Now the accessGrading() is called. This displays the expected message when the Grader upducks the post/reply.

Screencast.from.24-01-25.12.42.49.PM.IST.webm

Other information?

@codecov

codecov Bot commented Jan 24, 2025 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 22.09%. Comparing base (17035f6) to head (f220c81).
Report is 5 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##               main   #11358   +/-   ##
=========================================
  Coverage     22.09%   22.09%           
  Complexity     8839     8839           
=========================================
  Files           246      246           
  Lines         31810    31810           
  Branches         79       79           
=========================================
  Hits           7029     7029           
  Misses        24705    24705           
  Partials         76       76
Flag Coverage Δ
autograder 21.60% <ø> (ø)
js 26.85% <ø> (ø)
migrator 100.00% <ø> (ø)
php 19.10% <ø> (ø)
python_submitty_utils 80.08% <ø> (ø)
submitty_daemon_jobs 90.94% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@williamschen23

williamschen23 commented Jan 24, 2025

Copy link
Copy Markdown
Contributor

everything works, would you be able to add cypress tests to test that this functionality does not break?

@JManion32

JManion32 commented Jan 24, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

I verified the issue in main and tested the changes in this PR. Everything is functioning as expected, and I didn't run into any issues.

@williamschen23

williamschen23 commented Jan 24, 2025

Copy link
Copy Markdown
Contributor

I verified the issue in main and tested the changes in this PR. Everything is functioning as expected, and I didn't run into any issues.

you should approve it instead of adding a comment

JManion32
JManion32 approved these changes Jan 24, 2025
View reviewed changes

@JManion32 JManion32 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I verified the issue in main and tested the changes in this PR. Everything is functioning as expected, and I didn't run into any issues.

@sujay-d07

sujay-d07 commented Jan 25, 2025

Copy link
Copy Markdown
Contributor Author

everything works, would you be able to add cypress tests to test that this functionality does not break?

can you just tell me how exactly I can do that, I am currently not familiar with how to create them, then I could add the cypress tests.

@williamschen23

williamschen23 commented Jan 25, 2025

Copy link
Copy Markdown
Contributor

everything works, would you be able to add cypress tests to test that this functionality does not break?

can you just tell me how exactly I can do that, I am currently not familiar with how to create them, then I could add the cypress tests.

https://submitty.org/developer/testing/cypress
forums.spec.js starting from lines 181 to 200 has examples

williamjallen
williamjallen requested changes Jan 26, 2025
View reviewed changes

@williamjallen williamjallen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@williamschen23 is correct that we should have some Cypress tests for this functionality. Please test with students, limited access graders, full access graders, and instructors to make sure the functionality works, and continues to work, for all access levels.

@sujay-d07

sujay-d07 commented Jan 27, 2025

Copy link
Copy Markdown
Contributor Author

@williamschen23 is correct that we should have some Cypress tests for this functionality. Please test with students, limited access graders, full access graders, and instructors to make sure the functionality works, and continues to work, for all access levels.

Okay, will add Cypress tests for this.

@williamschen23

williamschen23 commented Feb 9, 2025

Copy link
Copy Markdown
Contributor

@williamjallen I'm doing more upducks refactor so it would make sense to add the grader's test to a new pr after the refactor is done, which I will do.

williamjallen
williamjallen approved these changes Feb 11, 2025
View reviewed changes

@williamjallen williamjallen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code looks good to me. @williamschen23 will write tests for this as part of the proposed refactor.

@bmcutler
bmcutler merged commit 729ffcd into Submitty:main Feb 12, 2025
lllinging pushed a commit to lllinging/Submitty that referenced this pull request Feb 14, 2025
@sujay-d07 @williamschen23 @lllinging
[Bugfix:Forum] Grader upduck message correction (Submitty#11358)
4cfda8b 
### Please check if the PR fulfills these requirements:

* [ ] Tests for the changes have been added/updated (if possible)
* [ ] Documentation has been updated/added if relevant
* [ ] Screenshots are attached to Github PR if visual/UI changes were
made

### What is the current behavior?
<!-- List issue if it fixes/closes/implements one using the "Fixes
#<number>" or "Closes #<number>" syntax -->
Closes Submitty#11355 

When the Grader upducked any thread or reply, there is the "liked by
teaching staff" message but when the page is refreshed the message gets
hidden as the accessFullGrading() was getting called, this prevented the
message to be displayed when a Grader upducked the post/reply.

[Screencast from 24-01-25 01:15:14 PM
IST.webm](https://github.com/user-attachments/assets/cc703979-e53f-40ed-8f7b-22fd06e4d728)

### What is the new behavior?
Now the accessGrading() is called. This displays the expected message
when the Grader upducks the post/reply.

[Screencast from 24-01-25 12:42:49 PM
IST.webm](https://github.com/user-attachments/assets/71d3e7ea-3b17-43e1-a86b-da59bf6eb83f)

### Other information?
<!-- Is this a breaking change? -->
<!-- How did you test -->

Co-authored-by: Williams Chen <[email protected]>
This was referenced Mar 31, 2026
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@williamjallen williamjallen williamjallen approved these changes
@JManion32 JManion32 JManion32 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

Submitty Development
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Grader upduck displaying wrong information

5 participants

@sujay-d07 @williamschen23 @JManion32 @williamjallen @bmcutler