Problem/Motivation

commerce_customer_profile_can_delete() is used to unset the $profiles array key but then if there are profiles in the array that are deletable $profiles is not empty and it deletes them anyway.

Proposed resolution

Unset also from the $profile_ids array.

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#12 2460491-12.patch3 KBjoelpittet
#12 interdiff.txt1.45 KBjoelpittet
#1 2460491-1-profile-with-order-delete.patch2.26 KBjoelpittet
#1 2460491-1-tests-only.patch1.6 KBjoelpittet

Comments

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Active » Needs review
StatusFileSize
new 2460491-1-tests-only.patch1.6 KB
new 2460491-1-profile-with-order-delete.patch2.26 KB

Here's with tests only and with a fix.

The last submitted patch, 1: 2460491-1-tests-only.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 1: 2460491-1-profile-with-order-delete.patch, failed testing.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented

Wrong one failed, that is bizarre, I guess I'll need to step through this.

Status: Needs work » Needs review

joelpittet queued 1: 2460491-1-profile-with-order-delete.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 1: 2460491-1-profile-with-order-delete.patch, failed testing.

rszrama’s picture

rszrama commented
Issue tags: +sprint

Tagging for http://contribkanban.com/#/board/commerce/7.x-1.x.

joelpittet queued 1: 2460491-1-tests-only.patch for re-testing.

Status: Needs work » Needs review

joelpittet queued 1: 2460491-1-profile-with-order-delete.patch for re-testing.

The last submitted patch, 1: 2460491-1-tests-only.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 1: 2460491-1-profile-with-order-delete.patch, failed testing.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt1.45 KB
new 2460491-12.patch3 KB

Hopeful retests:( Realized I'm dumb the profile_ids aren't keyed by profile_id. DOH!

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented

The tests only patch is still in #1 hasn't changed so if this is green. We win!

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Status: Needs review » Reviewed & tested by the community

Applies, prevents deletion of profiles. Only if all profiles passed failed to pass commerce_customer_profile_can_delete() would nothing get deleted. Even if one did all would be removed.

Fork extra kicks ran the tests on my GitHub+Travis fork https://travis-ci.org/mglaman/commerce-d7-travis/builds/72356123 with a patch branch.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Priority: Normal » Major

Has tests, thanks @mglaman bumping priority.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Issue tags: -sprint +Commerce Sprint
mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Status: Reviewed & tested by the community » Fixed

Thanks, and committed!

  • mglaman committed da8bdee on 7.x-1.x authored by joelpittet 
    Issue #2460491 by joelpittet: CommerceCustomerProfileEntityController::...

Status: Fixed » Closed (fixed)

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