Problem/Motivation

Should shipment also so updated for state machine update #3051503: Implement a confirmation form before the transition? Similar to what was done for orders in #2912849: Require confirmation for state transitions?

Comments

Neograph734 created an issue. See original summary.

jsacksick’s picture

Status: Active » Needs review
StatusFileSize
new2.5 KB

I'm expecting some tests to fail... But in any case, the attached patch cannot be committed as is... Unless we define our own state transition form (which might be necessary to fix the question that looks a bit ugly)... or we wait for a State machine release to be tagged to include the fix from #3220111: Url route parameters should be merged when redirecting to the confirmation form, otherwise the redirect to the confirmation form crashes due to the missing "commerce_order" parameter.

Status: Needs review » Needs work

The last submitted patch, 2: 3219675-2.patch, failed testing. View results

jsacksick’s picture

Status: Needs work » Needs review
StatusFileSize
new3.47 KB

We need to fix the question (i.e defining our own state transition form and extend getQuestion() but not sure what the question template should be...

The "question" currently says: "Are you sure you want to finalize shipment Shipment #1?"

Wondering if we shouldn't at least, add quotes around the shipment label 'Are you sure you want to finalize shipment "Shipment #1?"'..

Tests should pass now though.

neograph734’s picture

I noticed you already tagged a new state machine release, but I ran into that same issue during translating to Dutch; the order of words is different then. So perhaps better to still fix that in state machine?

The closest I got in the translation was something like "are you sure you want to apply transition 'x' to 'entity label y'?"

Sorry for insisting differently earlier... :(

jsacksick’s picture

I noticed you already tagged a new state machine release, but I ran into that same issue during translating to Dutch; the order of words is different then. So perhaps better to still fix that in state machine?

Well, whatever we come up with, I don't think we'll find a template that works correctly for all entity types.

I tagged a State machine release because a Commerce release is planned for next wednesday, could have waited a few more days, but wanted to start refactoring commerce_shipping etc.

In any event, this can be altered via code, and the confirmation can be disabled still... So I don't believe we'd need to rush yet another State machine release.

neograph734’s picture

I understand and agree with you. I will file the issue in state machine for later though.

jsacksick’s picture

StatusFileSize
new4 KB

Status: Needs review » Needs work

The last submitted patch, 8: 3219675-8.patch, failed testing. View results

jsacksick’s picture

Status: Needs work » Needs review
StatusFileSize
new4.68 KB
jsacksick’s picture

Title: Update the shipping entity to work with latest state machine? » Require confirmation before applying state transition for shipments
Status: Needs review » Fixed

Tests are green, committing!

  • jsacksick committed 542be75 on 8.x-2.x
    Issue #3219675 by Neograph734, jsacksick: Require confirmation before...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.