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

remove unused action option from BE #2764

Merged
merged 10 commits into from
Aug 19, 2024
Merged

remove unused action option from BE #2764

merged 10 commits into from
Aug 19, 2024

Conversation

samoddball
Copy link
Contributor

@samoddball samoddball commented Aug 15, 2024

NOREF

Description

  • progress towards rock - removing REST in favor of GraphQL
  • this is a cheap one cause it is removal only, but this seems safe to remove as we have this in the UI as the only usage of postAction:
postAction({
    intakeId: businessCase.systemIntakeId,
    actionType: isFinal
        ? 'SUBMIT_FINAL_BIZ_CASE'
        : 'SUBMIT_BIZ_CASE'
})

and we had this in the BE:

actionHandler := handlers.NewActionHandler(
    base,
    services.NewTakeAction(
        store.FetchSystemIntakeByID,
        map[models.ActionType]services.ActionExecuter{
            models.ActionTypeSUBMITBIZCASE: services.NewSubmitBusinessCase(
                // hidden for brevity
            ),
            models.ActionTypeSUBMITFINALBIZCASE: services.NewSubmitBusinessCase(
                // hidden for brevity
            ),
            models.ActionTypeSUBMITINTAKE: services.NewSubmitSystemIntake(
                // hidden for brevity
            ),
        },
    ),
)

because models.ActionTypeSUBMITINTAKE is not one of the actions sent through postAction, it seems fine to remove before we refactor business case to be completely graphql

How to test this change

PR Author Checklist

  • I have provided a detailed description of the changes in this PR.
  • I have provided clear instructions on how to test the changes in this PR.
  • I have updated tests or written new tests as appropriate in this PR.

PR Reviewer Guidelines

  • It's best to pull the branch locally and test it, rather than just looking at the code online!
  • When approving a PR, provide a reason why you're approving it
    • e.g. "Approving because I tested it locally and all functionality works as expected"
    • e.g. "Approving because the change is simple and matches the Figma design"
  • Don't be afraid to leave comments or ask questions, especially if you don't understand why something was done! (This is often a great time to suggest code comments or documentation updates)
  • Check that all code is adequately covered by tests - if it isn't feel free to suggest the addition of tests.

…. also, used `no-verify` because the go linter hangs. looking at that too
@samoddball samoddball changed the title (no verify) remove what appears to be an unused rest route - checking… remove unused action option from BE Aug 15, 2024
@samoddball samoddball marked this pull request as ready for review August 15, 2024 22:07
@samoddball samoddball requested a review from a team as a code owner August 15, 2024 22:07
@samoddball samoddball requested review from mynar7 and ClayBenson94 and removed request for a team August 15, 2024 22:07
@@ -16,7 +16,6 @@ import (

// NewFetchBusinessCaseByID is a service to fetch the business case by id
func NewFetchBusinessCaseByID(
config Config,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

unused

@samoddball samoddball requested a review from a team as a code owner August 16, 2024 22:32
@samoddball samoddball requested review from adamodd and removed request for a team August 16, 2024 22:32
Comment on lines -62 to -64
actionType: isFinal
? 'SUBMIT_FINAL_BIZ_CASE'
: 'SUBMIT_BIZ_CASE'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

no longer needed - only one function on backend regardless of which action

Copy link
Contributor

@mynar7 mynar7 left a comment

Choose a reason for hiding this comment

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

Nice, can't wait to nix the rest of this old REST code.

@samoddball samoddball merged commit 9f0b918 into main Aug 19, 2024
12 checks passed
@samoddball samoddball deleted the NOREF/rest_to_gql branch August 19, 2024 20:47
ClayBenson94 added a commit that referenced this pull request Sep 9, 2024
samoddball pushed a commit that referenced this pull request Sep 9, 2024
Revert "remove unused action option from BE (#2764)"

This reverts commit 9f0b918.
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.

2 participants