Problem/Motivation

While working on the customers and address migration, I came across the issue where there might not be a Drupal user corresponding to the profile being imported. The preSave() function makes a call to the loadByUser() which expects the user object to be passed. For this particular scenario, the user does not exist yet. This might not be a generic scenario - so not marking this as a bug.

Proposed resolution

Check if the account exists.

Remaining tasks

Create a patch. Review.

User interface changes

None.

API changes

None.

Data model changes

None.

CommentFileSizeAuthor
#13 3080025-11.patch2.24 KBkeerthana13
#10 3080025-10-test-and-fix.patch1.68 KBabramm
#10 3080025-10-test-only-failure-expected.patch847 bytesabramm
#7 interdiff_2-7.txt3.06 KBneelam_wadhwani
#7 3080025-7.patch1.65 KBneelam_wadhwani
#2 3080025-2.patch876 bytesajits

Comments

AjitS created an issue. See original summary.

ajits’s picture

ajits
he/him
Primary language Marathi
Location India
commented
StatusFileSize
new 3080025-2.patch876 bytes
ajits’s picture

ajits
he/him
Primary language Marathi
Location India
commented
Status: Active » Needs review
ajits’s picture

ajits
he/him
Primary language Marathi
Location India
commented
ajits’s picture

ajits
he/him
Primary language Marathi
Location India
commented
Issue tags: +Needs reroll

The patch is no longer applicable.

bojanz’s picture

bojanz commented
Status: Needs review » Needs work

Updating status. There's also postSave() to think about.

neelam_wadhwani’s picture

neelam_wadhwani commented
StatusFileSize
new 3080025-7.patch1.65 KB
new interdiff_2-7.txt3.06 KB

Hello,

For current profile check in postsave() and assigned it as default for current profile.
Attach is patch.

neelam_wadhwani’s picture

neelam_wadhwani commented
Status: Needs work » Needs review
abramm’s picture

abramm
Primary language Ukrainian
Location Lutsk
commented
Status: Needs review » Needs work

This is actually a bug since this code in preSave checks for profile owner existence by calling getOwnerId() and assuming that getOwner() would return non-null value as well:

    // Mark the profile as default if there's no other default.
    if ($this->getOwnerId() && $this->isPublished() && !$this->isDefault()) {
      $profile = $storage->loadByUser($this->getOwner(), $this->bundle());
      if (!$profile || !$profile->isDefault()) {
        $this->setDefault(TRUE);
      }
    }

However, this is not the case since getOwnerId() returns an ID of referenced user which may not exist (i.e. the user was deleted).
On the other hand, getOwnerId() may return 0 (meaning user is anonymous) so the correct approach would be checking both getOwnerId() (i.e. profile references some non-anonymous user) and getOwner() (profile references existing user).

abramm’s picture

abramm
Primary language Ukrainian
Location Lutsk
commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
StatusFileSize
new 3080025-10-test-only-failure-expected.patch847 bytes
new 3080025-10-test-and-fix.patch1.68 KB

Unfortunately I don't have working PHPUnit environment at the moment so I've added test changes relying on Drupal.org CI.

Attaching two patches, one having test only (expected to fail, demonstrates the bug) and the second one with test + fix.

The last submitted patch, 10: 3080025-10-test-only-failure-expected.patch, failed testing. View results

abramm’s picture

abramm
Primary language Ukrainian
Location Lutsk
commented
Category: Task » Bug report
keerthana13’s picture

keerthana13 commented
StatusFileSize
new 3080025-11.patch2.24 KB

Getting null in loadMultipleByUser() as well. so rerolled patch #10

abramm’s picture

abramm
Primary language Ukrainian
Location Lutsk
commented

Hi @keerthana13 ,
Can you please post an interdiff as well as two patches (test only which should fail and test+fix which should pass)?

That would help adopting the patch.
Thanks!

  • jsacksick committed acedd50 on 8.x-1.x 
    Issue #3080025 by abramm, neelam_wadhwani, AjitS, keerthana13, bojanz,...
jsacksick’s picture

jsacksick commented
Status: Needs review » Fixed

Committed the patch from #13, thanks everyone!

  • jsacksick committed dfbf75a on 8.x-1.x 
    Issue #3080025 followup: Put back the getOwnerId() check in Profile::...

Status: Fixed » Closed (fixed)

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