Skip to content

[Refactor:SubminiPolls] Move poll access to Access.php#11638

Merged
bmcutler merged 9 commits intomainfrom
polls-refactor
Jun 30, 2025
Merged

[Refactor:SubminiPolls] Move poll access to Access.php#11638
bmcutler merged 9 commits intomainfrom
polls-refactor

Conversation

@lavalleeale
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?

When viewing a poll, the authorization logic is handled within the polls controller which makes it inaccessible to anywhere else in the codebase that might need to verify if a user should have access to a poll.

What is the new behavior?

The logic is moved to Access.php under poll.view and poll.view.histogram

Other information?

Needed for #11634

@codecov
Copy link
Copy Markdown

codecov bot commented May 28, 2025

Codecov Report

Attention: Patch coverage is 40.00000% with 12 lines in your changes missing coverage. Please review.

Project coverage is 20.69%. Comparing base (c8c0494) to head (d6c1562).
Report is 61 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##               main   #11638      +/-   ##
============================================
- Coverage     21.04%   20.69%   -0.35%     
- Complexity     9077     9182     +105     
============================================
  Files           258      263       +5     
  Lines         34866    35228     +362     
  Branches        461      461              
============================================
- Hits           7337     7290      -47     
- Misses        27072    27481     +409     
  Partials        457      457              
Flag Coverage Δ
autograder 21.34% <ø> (-0.10%) ⬇️
js 2.11% <ø> (-2.54%) ⬇️
migrator 100.00% <ø> (ø)
php 19.38% <40.00%> (-0.19%) ⬇️
python_submitty_utils 80.08% <ø> (ø)
submitty_daemon_jobs 88.88% <ø> (ø)

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@lavalleeale lavalleeale requested a review from williamjallen May 29, 2025 14:39
@automateprojectmangement automateprojectmangement bot moved this from Seeking Reviewer to In Review in Submitty Development May 29, 2025
Copy link
Copy Markdown
Member

@williamjallen williamjallen left a comment

Choose a reason for hiding this comment

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

A few brief stylistic comments, otherwise looks reasonable to me.

@github-project-automation github-project-automation bot moved this from In Review to Work in Progress in Submitty Development May 30, 2025
lavalleeale and others added 4 commits June 13, 2025 12:15
Co-authored-by: William Allen <16820599+williamjallen@users.noreply.github.com>
@automateprojectmangement automateprojectmangement bot moved this from Work in Progress to In Review in Submitty Development Jun 13, 2025
Copy link
Copy Markdown
Member

@williamjallen williamjallen left a comment

Choose a reason for hiding this comment

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

Code seems reasonable to me.

@github-project-automation github-project-automation bot moved this from In Review to Awaiting Maintainer Review in Submitty Development Jun 13, 2025
@williamschen23 williamschen23 moved this from Awaiting Maintainer Review to Seeking Reviewer in Submitty Development Jun 13, 2025
@williamschen23 williamschen23 moved this from Seeking Reviewer to Work in Progress in Submitty Development Jun 13, 2025
Copy link
Copy Markdown
Member

@williamjallen williamjallen left a comment

Choose a reason for hiding this comment

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

As discussed in the meeting today, let's add a unit test for this before merge.

Copy link
Copy Markdown
Member

@williamjallen williamjallen left a comment

Choose a reason for hiding this comment

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

Thanks for adding a unit test!

@github-project-automation github-project-automation bot moved this from Work in Progress to Awaiting Maintainer Review in Submitty Development Jun 27, 2025
@williamjallen williamjallen moved this from Awaiting Maintainer Review to Ready to Merge in Submitty Development Jun 27, 2025
@bmcutler bmcutler merged commit 84cf48d into main Jun 30, 2025
24 checks passed
@bmcutler bmcutler deleted the polls-refactor branch June 30, 2025 02:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants