Skip to content

Conversation

@fogelito
Copy link
Contributor

@fogelito fogelito commented Oct 1, 2023

No description provided.

fogelito and others added 2 commits October 2, 2023 10:40
Comment on lines +1540 to 1549
try {
if ($attribute->getAttribute('type') === self::VAR_RELATIONSHIP
&& \strtolower($attribute->getAttribute('options')['twoWayKey']) === \strtolower($twoWayKey)
&& $attribute->getAttribute('options')['relatedCollection'] === $relatedCollection->getId()
) {
throw new DuplicateException('Related attribute already exists');
}
} catch (DuplicateException $e) {
$twoWayKey ??= $collection->getId() . '_' . uniqid();
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need a try/catch here, we can use use the if:

Suggested change
try {
if ($attribute->getAttribute('type') === self::VAR_RELATIONSHIP
&& \strtolower($attribute->getAttribute('options')['twoWayKey']) === \strtolower($twoWayKey)
&& $attribute->getAttribute('options')['relatedCollection'] === $relatedCollection->getId()
) {
throw new DuplicateException('Related attribute already exists');
}
} catch (DuplicateException $e) {
$twoWayKey ??= $collection->getId() . '_' . uniqid();
}
if ($attribute->getAttribute('type') === self::VAR_RELATIONSHIP
&& \strtolower($attribute->getAttribute('options')['twoWayKey']) === \strtolower($twoWayKey)
&& $attribute->getAttribute('options')['relatedCollection'] === $relatedCollection->getId()
) {
$twoWayKey ??= $collection->getId() . '_' . uniqid();
}

throw new DuplicateException('Related attribute already exists');
}
} catch (DuplicateException $e) {
$twoWayKey ??= $collection->getId() . '_' . uniqid();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be $twoWayKey = $collection->getId() . '_' . uniqid();, otherwise it will only assign if null

@fogelito
Copy link
Contributor Author

We tool care of this on Appwrite side

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants