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

Refactor request spec test #4147

Merged
merged 4 commits into from
Mar 8, 2024

Conversation

elasticspoon
Copy link
Collaborator

@elasticspoon elasticspoon commented Feb 29, 2024

Depends on #4142

Description

Moves a bunch of behavior from managing_requests_system_spec.rb to request specs. A lot of what the system test was doing was basically checking input validation, that can be done in requests spec much cheaper.

Type of change

  • Refactor

Fixes rubyforgood#4135

When individuals requests were submitted with incorrect values
the error page did not preserve correct or incorrect inputs. This
was happening because the controller created a new request on
error.

Now the new request will be populated with the items from the
failed request.

I attempted to reuse the failed request, however, because the
individuals request is a wrapper around a family request that
behavior was not possible.
@elasticspoon elasticspoon marked this pull request as ready for review March 4, 2024 03:17
@elasticspoon
Copy link
Collaborator Author

@cielf When I was refactoring I noticed that family requests require certain permissions:

module Partners
  class FamilyRequestsController < BaseController
    before_action :verify_partner_is_active
    before_action :authorize_verified_partners

whereas Individual requests don't:

module Partners
  class FamilyRequestsController < BaseController

  no callbacks here

is that intended or an oversight?

@cielf
Copy link
Collaborator

cielf commented Mar 4, 2024

From a business p.o.v. you have to be approved before making requests for all three of the different kinds of requests.

Copy link
Collaborator

@dorner dorner left a comment

Choose a reason for hiding this comment

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

Some tiny comments - I really love this! Not only the reduction in system tests (which are always awesome) but the tests are really easy to read and well organized. Kudos!

@elasticspoon elasticspoon requested a review from dorner March 7, 2024 20:46
@dorner
Copy link
Collaborator

dorner commented Mar 8, 2024

Thank you!

@dorner dorner merged commit d386a4e into rubyforgood:main Mar 8, 2024
18 of 19 checks passed
@elasticspoon elasticspoon deleted the refactor-request-spec-test branch March 8, 2024 20:46
Copy link
Contributor

@elasticspoon: Your PR Refactor request spec test is part of today's Human Essentials production release: 2024.03.17.
Thank you very much for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants