[mrafie1]

Proposed Changes

Summary: Previously, in the results_controller.rb file, the functions add_tag and remove_tag used Tag.find to search for tags to add/remove. This causes a bug when a tag that doesn't belong to the course of the item result we are looking at is added/removed.

Changes:

• Edited these two functions to use current_course.tags to search for the tags available in the current course that have the same assessment id (or none at all).
• Edited the test to confirm the bug fix prevents tags from other courses from being used in add_tag and remove_tag.

Oct 28 Changes

• Updated results_controller_spec.rb file to utilize a shared example block (shared ta and instructor tests)
• Updated tests that were not working as a result of the shared example block being run
  • update_mark test:
    • %w[total subtotal mark_override num_marked mark] block
      • it was a stale test. I added the keyword, "last_updated_by", which is now returned in the controller response
    • it { expect(response).to have_http_status(:redirect) } block
      • The correct status for this request was 200 OK, not a redirect.
    • 'Duplicate marks exist' test
      • During the setup of the test, Postgres raised ActiveRecord::RecordNotUnique PG::UniqueViolation error since we tried to add a second mark with the sameresult_id criterion_id. We fixed this by using test doubles to simulate the race condition, stub the duplicate-cleanup query, and verify that destroy_all is called.

Screenshots of your changes (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)	x
🎨 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)
📦 Dependency update (change that updates a dependency)
📖 Documentation update (change that updates documentation)
🔧 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.)

[coveralls]

Coverage Report for CI Build 40896731321

Coverage increased (+0.4%) to 91.462%

Details

• Coverage increased (+0.4%) from the base build.
• Patch coverage: 43 of 43 lines across 2 files are fully covered (100%).
• No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.

---

Coverage Stats

Relevant Lines:	54174
Covered Lines:	50606
Line Coverage:	93.41%
Relevant Branches:	2569
Covered Branches:	1292
Branch Coverage:	50.29%
Branches in Coverage %:	Yes
Coverage Strength:	129.42 hits per line

---

💛 - Coveralls

[david-yz-liu]

Looking at the coverage report, it may be that the shared_examples block that you added to is never actually run. (See https://rspec.info/features/3-13/rspec-core/example-groups/shared-examples/)

This isn't great; please go ahead and address that in this PR.

[david-yz-liu]

I think we can be even more specific in our query by limiting our search to tags where the assessment_id matches the grouping's assessment_id or is nil.

(Same applies below)

[mrafie1]

Changes:

• result_controller_spec.rb now runs the shared test block of shared ta and instructor tests for both instructors and ta's.
• changes to some tests were made as their expected responses were stale. more details can be found in the PR description.

[david-yz-liu]

Nice work, @mrafie1!