Skip to content

Fix common db relation insertion #439

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
stonebuzz merged 4 commits into from
Dec 17, 2024
Merged

Fix common db relation insertion #439

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9c5735f
Fix common db relation insertion
Lainow Nov 19, 2024
8acb323
Fix and update changelog
Lainow Dec 11, 2024
bc281dd
Update CHANGELOG.md
Lainow Dec 11, 2024
69121b9
Merge branch 'main' into fix-commondbrelation-injection
Lainow Dec 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion inc/commoninjectionlib.class.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1606,7 +1606,7 @@ private function effectiveAddOrUpdate($injectionClass, $item, $values, $add = tr
//CommonDBRelation are managed separately, so related field should be ignored
// Ex : User -> groups_id -> Group_User
// groups_id should not be injected in User (field contains group name (string))
if ($option !== false && isset($option['displaytype']) && $option['displaytype'] == 'relation') {
if ($option !== false && isset($option['displaytype']) && $option['displaytype'] == 'relation' && !($item instanceof CommonDBRelation)) {
Copy link
Contributor

@stonebuzz stonebuzz Nov 28, 2024

Choose a reason for hiding this comment

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

I'm not sure about the fix, with this example (inject groups_id from User object)

User -> groups_id (from Group_User)

The condition will return false (to delegate to specific case) -> OK

But with this

User -> name

The condition will return false -> KO

Because User is not an instance of CommonDBRelation

Suggested change
if ($option !== false && isset($option['displaytype']) && $option['displaytype'] == 'relation' && !($item instanceof CommonDBRelation)) {
if (($option !== false && isset($option['displaytype']) && $option['displaytype'] == 'relation') || !($item instanceof CommonDBRelation)) {

Copy link
Contributor Author

@Lainow Lainow Dec 3, 2024

Choose a reason for hiding this comment

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

With the || operator, the plugin doesn't insert any data, whereas with &&, all data is actually inserted.

Copy link
Contributor Author

@Lainow Lainow Dec 3, 2024

Choose a reason for hiding this comment

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

And I tested with a user like you said but everything works perfectly.

Copy link
Contributor

@stonebuzz stonebuzz Dec 3, 2024

Choose a reason for hiding this comment

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

👍

continue;
}

Expand Down