Fix: submitSucceeded is set on redux-form/STOP_SUBMIT

[stefan-dimitrov]

When doing a submit the event flow was:

redux-form/START_SUBMIT
  submitting: true
redux-form/STOP_SUBMIT
  submitting: undefined
  submitSucceeded: true // this should not be here
redux-form/SET_SUBMIT_FAILED
  submitSucceeded: undefined
  submitFailed: true

So on a failed submit we would get a submitSucceeded flag set before we get the submitFailed, which made it very difficult to reliably determine a failure.

With this change the submitSucceeded is no longer set with the STOP_SUBMIT action, but only with the dedicated SET_SUBMIT_SUCCEEDED.

Fixes #2260

[cdimitroulas]

👍 just came across this issue myself and this PR would allow reliance on the submitSucceeded prop which otherwise does not work correctly

[ssilve1989]

I would also like to see this merged. The current behavior of submitSucceeded makes it impossible to rely on it to do any sensible flow in managing form component rendering.

[codecov-io]

Codecov Report

❗ No coverage uploaded for pull request base (master@b5240aa). Click here to learn what that means.
The diff coverage is 100%.

@@           Coverage Diff            @@
##             master   #3830   +/-   ##
========================================
  Coverage          ?    100%           
========================================
  Files             ?      70           
  Lines             ?    1601           
  Branches          ?       0           
========================================
  Hits              ?    1601           
  Misses            ?       0           
  Partials          ?       0

Impacted Files	Coverage Δ
src/createReducer.js	100% <100%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b5240aa...7b59fb7. Read the comment docs.

[erikras]

Published in v7.3.0.

[8of]

So, how should we use it from now on if we need submitSucceeded?
Should we call setSubmitSucceeded(formId) instead of stopSubmit(formId) if our submitting was successful?

[cdimitroulas]

@8of you shouldn't need to call any of redux-form's action creators or to do anything special.

redux-form passes a prop called submitSucceeded to your wrapped component which will change to true once the form has submitted successfully. This PR was to fix an issue where it was changing to true even though the form did not submit successfully.

Unless I'm misunderstanding your question...?

[8of]

@cdimitroulas
If I execute setSubmitSucceeded(formId) then submitting is still true, and it shouldn't be true, because, well, submitting was successful.
Any suggestions?

[cdimitroulas]

hmm that's odd - shouldn't be the case. @stefan-dimitrov should the reducer also set submitting to false when the submission succeeds?

@8of for now as a workaround you could maybe check that submitting is true AND submitSucceeded is false?

[stefan-dimitrov]

should the reducer also set submitting to false when the submission succeeds?

I believe it should not. My understanding is that those actions should set only their respective flags, without affecting others, even if they are "thematically" related (submit completion and submit success).
redux-form handles setting of those flags automatically when the submission completes (probably using the Promise resolution mechanism, I haven't looked into those details).

I could be wrong, but what you are describing, @8of, seems to be manual setting of the flags, without relying on the mechanisms in redux-form. If that is case I think the setting of the related flags should be handled explicitly by your logic.
I do not think there is any missing behavior in redux-form regarding your scenario.

[8of]

those actions should set only their respective flags
Makes sense.
Now I make calls like this:

stopSubmit(formId);
setSubmitSucceeded(formId);

submitting and submitSucceeded have expected values (false and true respectively)

The only downside I can think of - submitting and submitSucceeded don't switch states at the same time. But that's ok for me.

Thank you guys for the clarification. 👏

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.