Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add more comprehensive testing for publications controller #338

Merged
merged 5 commits into from
Mar 11, 2024

Conversation

Janell-Huyck
Copy link
Contributor

refs #337

This PR gives more comprehensive testing for the PublicationsController. It is the first PR for issue #337. Later PRs will involve creating a superficial CRUD testing shared examples for the different publication types and removing the redundant testing for those controllers.

Since we do not have a Publications model, and since all of the publication types inherit their CRUD methods directly from the parent controller, we are testing the PublicationsController through its direct child controller, BooksController. We are testing for user authentication and the Create, Index, Update, and Destroy methods defined in PublicationsController.

The file spec/support/helpers/login_helpers_for_unit_tests.rb was created in PR #323, but as that has not been merged in at the time of the PR creation, I have recreated it here in this PR. I've added a login_as_admin method in the helper and added a line that will log out any admin user from an admin role when they are logging in as a submitter of a work.

@Janell-Huyck Janell-Huyck changed the title Add more comprehensive testing for publications controller 337 - Add more comprehensive testing for publications controller Jan 18, 2024
Copy link
Member

@hortongn hortongn left a comment

Choose a reason for hiding this comment

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

@Janell-Huyck This all looks great. I appreciate the comments you included. My only suggestion is a spec for #show that verifies @submitter is getting set properly for an admin session. Push back if you think that's covered in other ways.

@Janell-Huyck
Copy link
Contributor Author

That's a good idea. @submitter and @submitters is getting set differently for admins and submitters in this logic. I'll get a check on that and let you know when I'm ready for a re-check.

@Janell-Huyck Janell-Huyck changed the title 337 - Add more comprehensive testing for publications controller WIP 337 - Add more comprehensive testing for publications controller Feb 12, 2024
@Janell-Huyck Janell-Huyck force-pushed the 337-detailed-tests-for-publications-controller branch 2 times, most recently from 47c5d4a to 0faee0a Compare February 20, 2024 18:33
@Janell-Huyck Janell-Huyck force-pushed the 337-detailed-tests-for-publications-controller branch from 0faee0a to bb85d37 Compare February 21, 2024 18:25
@Janell-Huyck Janell-Huyck changed the title WIP 337 - Add more comprehensive testing for publications controller Add more comprehensive testing for publications controller Feb 21, 2024
@Janell-Huyck Janell-Huyck changed the title Add more comprehensive testing for publications controller WIP Add more comprehensive testing for publications controller Feb 21, 2024
@Janell-Huyck Janell-Huyck changed the title WIP Add more comprehensive testing for publications controller Add more comprehensive testing for publications controller Feb 21, 2024
@Janell-Huyck Janell-Huyck force-pushed the 337-detailed-tests-for-publications-controller branch from 31a8a7a to ea56fd3 Compare February 27, 2024 17:51
@Janell-Huyck Janell-Huyck force-pushed the 337-detailed-tests-for-publications-controller branch from ea56fd3 to 926f982 Compare March 6, 2024 17:18
Copy link
Member

@hortongn hortongn left a comment

Choose a reason for hiding this comment

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

@Janell-Huyck Looks great. Thanks for refactoring PublicationsController#show

@hortongn hortongn merged commit 6af1856 into qa Mar 11, 2024
2 checks passed
@hortongn hortongn deleted the 337-detailed-tests-for-publications-controller branch March 11, 2024 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants