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

Treat FlowType=AbandonCurrentMoveToNext as reject action #117

Merged
merged 4 commits into from
Oct 10, 2024

Conversation

danielskovli
Copy link
Contributor

@danielskovli danielskovli commented Oct 8, 2024

Description

Due to a patch in Altinn/altinn-storage#414, ProcessController treats FlowType=AbandonCurrentMoveToNext identically to FlowType=CompleteCurrentMoveToNext. This has now proven to be problematic, as described in Altinn/app-lib-dotnet#818.

This PR intercepts AbandonCurrentMoveToNext and treats it as reject instead of whatever altinn:taskType from the current step was.

NOTE: This functionality must be implemented in altinn-storage as well. This will be the next step, after we have confirmed functionality and reached agreement on how it should look in localtest.

Related Issue(s)

Verification

  • Your code builds clean without any errors or warnings
  • Manual testing done (required)
  • Relevant automated test added (if you find this hard, leave it and we'll help out)
  • All tests run green

Documentation

  • User documentation is updated with a separate linked PR in altinn-studio-docs. (if applicable)

Instead of `altinn:taskType` from current step in BPMN
@danielskovli danielskovli added the kind/bug Something isn't working label Oct 8, 2024
@danielskovli danielskovli self-assigned this Oct 8, 2024
@danielskovli danielskovli changed the title Treat FlowType=AbandonCurrentMoveToNext as data action Treat FlowType=AbandonCurrentMoveToNext as reject action Oct 9, 2024
@danielskovli danielskovli marked this pull request as ready for review October 9, 2024 10:11
@danielskovli
Copy link
Contributor Author

App developer has reported back that this patch resolves their issue.

@SandGrainOne
Copy link
Member

I don't want the implementation in app localtest to deviate too much from the implementation in Storage. Is this fix unnecessary for an app running in production?

@danielskovli
Copy link
Contributor Author

@SandGrainOne This fix definitely needs to be implemented in Storage as well. But seeing as they are two different implementations, I just had to start somewhere. If I get approval here, I will produce the same patch for Storage as a separate PR.

I should have communicated this more clearly.

@danielskovli danielskovli merged commit 2823e2a into main Oct 10, 2024
4 checks passed
@danielskovli danielskovli deleted the fix/process-reject-action branch October 10, 2024 07:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants