Skip to content
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.

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

[Bug]: setUserModification in SimpleBackendSearchBundle does not accept null values #16965

Closed
gfemorris opened this issue Apr 19, 2024 · 13 comments
Labels

Comments

@gfemorris
Copy link

gfemorris commented Apr 19, 2024

Pimcore version

11.2.2

Steps to reproduce

If backend user gets deleted userModification and or userOwner is nulled in elements. The Backendsearch crashes if there is a null value in items.

Actual Behavior

This happens in Pimcore\Bundle\SimpleBackendSearchBundle\Model\Search\Backend\Data:setUserModification / setUserOwner which accepts only integer values.

Same happens with object folders when you try to save them.
In this case it happens in
Pimcore\Bundle\AdminBundle\Controller\Admin\DataObject\DataObjectController:saveFolderAction
when setValues is called. The null value of the userModification /userOwner is included and crashes in AbstractElement because it does not accept null values as well.

Expected Behavior

Method should accept null values as well or typecasting before setting it.

@blankse
Copy link
Contributor

blankse commented Apr 21, 2024

@gfemorris I created a PR #16970

@gfemorris
Copy link
Author

@blankse thx. That should fix the search case. The issue with the object folder is still open though. Should i create a new issue for that so we can close this one?

@blankse
Copy link
Contributor

blankse commented Apr 26, 2024

@gfemorris I can't reproduce this. In my setup the timestamps are set:
Screenshot 2024-04-26 132144

How can this values null?

@gfemorris
Copy link
Author

@blankse The browser sends null if there is no userModification set
grafik
grafik
And then it crashes
grafik

@blankse
Copy link
Contributor

blankse commented Apr 29, 2024

@gfemorris Thank you.
I changed the 4 attributes in the db to null. Than I can't save the folder. I added a commit to the PR to fix this.

@lukmzig
Copy link
Contributor

lukmzig commented May 3, 2024

Hi @blankse @gfemorris
just for my understanding, can it actually happen that all 4 values

  • userModification
  • userOwner
  • creationDate
  • modificationDate
    are null?

Because in your example you are mentioning only userModification. Thank you.

@blankse
Copy link
Contributor

blankse commented May 3, 2024

The db columns are nullable. So it is possible. Maybe not via admin interface. But via php script.

@lukmzig
Copy link
Contributor

lukmzig commented May 3, 2024

@blankse if we change the setters to accept the null values then yes, these fields will be null-able via script. However, then this means we have empty data in the DB.

I will discuss with my colleagues this topic, for me the better solution would be to either assign system user when this happens and introduce some mechanism to assign system user on deleting of existing users.

@blankse
Copy link
Contributor

blankse commented May 3, 2024

@lukmzig The null value could be exists from a earlier pimcore version without this type hints (Pimcore < 11) or someone set it directly in the db table via sql.

So if you decide to make it not nullable, the db schema should be changed and the data should be migrated.

I think there should be a difference between system user and deleted user. The system user is for changes of pimcore itself (via maintenance cronjob for example). For the created and modified dates I don't know a reason to be empty. So there we could change the db shema to not nullable and default to current time when empty it is a problem for you.

@lukmzig
Copy link
Contributor

lukmzig commented May 7, 2024

@blankse I agree with you, the DB schema should not change here. This issue with the folder is bit strange and we will need to have a deeper look

@gfemorris can you please create a separate issue for the folder this? Thank you very much!

@blankse
Copy link
Contributor

blankse commented May 7, 2024

@gfemorris @lukmzig I created a new issue: #17019

@lukmzig
Copy link
Contributor

lukmzig commented May 7, 2024

The SimpleBackenSearchBundle part was fixed by #16970
The folder part will be solved in separate issue

@lukmzig lukmzig closed this as completed May 7, 2024
@gfemorris
Copy link
Author

@lukmzig agreed. It's better to have a separate issue for this. Thx @blankse for creating the issue.
I am not really sure why we have folders with null values in userModification. My guess was that the client deleted the user that was in there so it went to null. Maybe that happened in an older version. Nonetheless it makes sense to fix that so that people will not have problems if they migrate to pimcore11.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants