Add tests for #new and #create in CriteriaController when marks are r… #7521

freyazjiner · 2025-05-11T16:55:27Z

Changes

(Describe your changes here. Also describe the motivation for your changes: what problem do they solve, or how do they improve the application or codebase? If this pull request fixes an open issue, use a keyword to link this pull request to the issue.)

Adds tests for the “marks released” edge-case to both #new and #create. The approach is first create an assignment with released results, makes the corresponding request (get_as or post_as), and verifies that the controller returns a flash message with an error and an appropriate response code.

Screenshots of your changes (if applicable)

Associated documentation repository pull request (if applicable)

Type of Change

(Write an X or a brief description next to the type or types that best describe your changes.)

Type	Applies?
🚨 Breaking change (fix or feature that would cause existing functionality to change)
✨ New feature (non-breaking change that adds functionality)
🐛 Bug fix (non-breaking change that fixes an issue)
🎨 User interface change (change to user interface; provide screenshots)
♻️ Refactoring (internal change to codebase, without changing functionality)
🚦 Test update (change that only adds or modifies tests)	X
📦 Dependency update (change that updates a dependency)
🔧 Internal (change that only affects developers or continuous integration)

Checklist

(Complete each of the following items for your pull request. Indicate that you have completed an item by changing the [ ] into a [x] in the raw text, or by clicking on the checkbox in the rendered description on GitHub.)

Before opening your pull request:

I have performed a self-review of my changes.
- Check that all changed files included in this pull request are intentional changes.
- Check that all changes are relevant to the purpose of this pull request, as described above.
I have added tests for my changes, if applicable.
- This is required for all bug fixes and new features.
I have updated the project documentation, if applicable.
- This is required for new features.
If this is my first contribution, I have added myself to the list of contributors.

After opening your pull request:

I have updated the project Changelog (this is required for all changes).
I have verified that the pre-commit.ci checks have passed.
I have verified that the CI tests have passed.
I have reviewed the test coverage changes reported by Coveralls.
I have requested a review from a project maintainer.

Questions and Comments

(Include any questions or comments you have regarding your changes.)
I split each controller action into two scenarios:

Marks not released – existing examples remain here.
Marks released – new examples that test the error-path.
Github tests all pass for now.

…eleased

coveralls · 2025-05-11T17:20:46Z

Pull Request Test Coverage Report for Build 15124982480

Details

33 of 33 (100.0%) changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.02%) to 91.933%

Totals
Change from base Build 14870512584:	0.02%
Covered Lines:	41867
Relevant Lines:	44856

💛 - Coveralls

david-yz-liu

@freyazjiner a good start. Here are a few general comments:

You deleted the pull request template that pre-populated the description field when making the pull request, which you should not do. You can find the pull request template under the .github folder; please use it to update your pull request description, and make sure to read through it and use the checklist to guide other things you need to do when making a pull request.
The tests should be added to the existing tests for #new and #create, rather than being created in a separate block. You can update just the context that involves "An authenticated and authorized instructor doing a GET". You'll need to figure out a good way to combine the setup required for these cases with the existing test cases.

adds Freya Zhang to the list

david-yz-liu

@freyazjiner Good work! I just left a few minor comments.

david-yz-liu · 2025-05-14T23:06:48Z

Changelog.md

@@ -12,6 +12,7 @@

 - Remove `activerecord-session_store` gem (#7517)
 - Upgrade to Rails 8 (#7504)
+- Tests added for #new and #create actions for CriteriaController (#7521)


For consistency with other entries, phrase this as "Add tests for..." (i.e., starting with a verb in the imperative tense).

Also use backticks to surround all code names (#new, #create, CriteriaController).

david-yz-liu · 2025-05-14T23:07:50Z

spec/controllers/criteria_controller_spec.rb

-                 params: { course_id: course.id, assignment_id: assignment.id },
-                 format: :js
-        end
+        context 'assignment marks are not released' do


For context blocks, the description should generally start with the word "when"

david-yz-liu · 2025-05-14T23:08:41Z

spec/controllers/criteria_controller_spec.rb

+            expect(flash[:error]).to have_message(I18n.t('criteria.errors.released_marks'))
+          end
+
+          it 'responds with appropriate status' do


Be more specific: responds with :bad_request status. Same comment below.

david-yz-liu · 2025-05-16T12:28:13Z

.github/PULL_REQUEST_TEMPLATE.md

@@ -1,7 +1,7 @@
 ## Proposed Changes
 *(Describe your changes here. Also describe the motivation for your changes: what problem do they solve, or how do they improve the application or codebase? If this pull request fixes an open issue, [use a keyword to link this pull request to the issue](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword).)*

-...
+Adds tests for the “marks released” edge-case to both `#new` (GET) and `#create` (POST). Each spec builds an assignment with released results, makes the corresponding request (`get_as` or `post_as`), and verifies that the controller flashes the correct error and returns `400 Bad Request`.


I think you accidentally committed changes to this file (you shouldn't change the template file itself, just make sure to fill it out on GitHub).

david-yz-liu · 2025-05-16T12:28:20Z

Changelog.md

@@ -6,12 +6,14 @@

 ### ✨ New features and improvements

+


Revert this change

sorry about that, done

This line is still here (you may have reverted this change by accident). To be clear, I'm referring to the blank line that has been added; there should be just one blank line after the "New features and improvements" heading.

david-yz-liu · 2025-05-19T20:08:24Z

Changelog.md

@@ -6,12 +6,14 @@

 ### ✨ New features and improvements

+


This line is still here (you may have reverted this change by accident). To be clear, I'm referring to the blank line that has been added; there should be just one blank line after the "New features and improvements" heading.

david-yz-liu · 2025-05-19T20:09:10Z

doc/markus-contributors.txt

@@ -91,7 +92,7 @@ Horatiu Halmaghi
 Huiyu Sun
 Ian Smith
 Ibrahim Shahin
-Ido Ben Haim
+Igit revert HEADdo Ben Haim


This line contains an error. Please make sure to check your work carefully when making commits, using git diff (and the GitHub pull request interface).

Add tests for #new and #create in CriteriaController when marks are r…

e0f57b4

…eleased

david-yz-liu reviewed May 13, 2025

View reviewed changes

freyazjiner and others added 3 commits May 13, 2025 00:00

merged released-marks scenarios into existing tests for #new and #create

662488e

deleted redundant parts

a06a438

Merge branch 'master' into test_new_and_create_freya

c2fbe90

freyazjiner requested a review from david-yz-liu May 13, 2025 19:56

freyazjiner added 2 commits May 13, 2025 16:03

Update Changelog.md

462a66a

Update markus-contributors.txt

fc33ec5

adds Freya Zhang to the list

david-yz-liu reviewed May 14, 2025

View reviewed changes

freyazjiner added 2 commits May 15, 2025 09:18

adds Freya Zhang to the list

0ce1ff3

updated some minor wording changes

a3d953b

david-yz-liu reviewed May 16, 2025

View reviewed changes

revert the change in PULL_REQUEST_TEMPLATE.md

0bf241c

freyazjiner requested a review from david-yz-liu May 17, 2025 20:35

freyazjiner added 4 commits May 17, 2025 16:39

improvement to the pull request template

229c683

fixed the link for list of contributors

dace833

revert wrong changes

73b1684

revert wrong changes

79058c4

david-yz-liu reviewed May 19, 2025

View reviewed changes

freyazjiner added 2 commits May 19, 2025 19:05

revert the new line added in line 8

2b02fb3

revert the change

f1544d1

david-yz-liu approved these changes May 20, 2025

View reviewed changes

david-yz-liu merged commit b80800e into MarkUsProject:master May 20, 2025
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add tests for #new and #create in CriteriaController when marks are r… #7521

Add tests for #new and #create in CriteriaController when marks are r… #7521

Uh oh!

freyazjiner commented May 11, 2025 •

edited

Loading

Uh oh!

coveralls commented May 11, 2025 •

edited

Loading

Uh oh!

david-yz-liu left a comment

Uh oh!

david-yz-liu left a comment

Uh oh!

david-yz-liu May 14, 2025

Uh oh!

david-yz-liu May 14, 2025

Uh oh!

david-yz-liu May 14, 2025

Uh oh!

david-yz-liu May 16, 2025

Uh oh!

david-yz-liu May 16, 2025

Uh oh!

freyazjiner May 17, 2025

Uh oh!

david-yz-liu May 19, 2025

Uh oh!

david-yz-liu May 19, 2025

Uh oh!

david-yz-liu May 19, 2025

Uh oh!

Uh oh!

Uh oh!

Add tests for #new and #create in CriteriaController when marks are r… #7521

Add tests for #new and #create in CriteriaController when marks are r… #7521

Uh oh!

Conversation

freyazjiner commented May 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Type of Change

Checklist

Questions and Comments

Uh oh!

coveralls commented May 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 15124982480

Details

💛 - Coveralls

Uh oh!

david-yz-liu left a comment

Choose a reason for hiding this comment

Uh oh!

david-yz-liu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

freyazjiner commented May 11, 2025 •

edited

Loading

coveralls commented May 11, 2025 •

edited

Loading