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

[Feature]: Automaticaly set imported element userModification #418

Merged
merged 4 commits into from
Aug 26, 2024

Conversation

root913
Copy link
Contributor

@root913 root913 commented Jul 16, 2024

This PR solves problem of identifying who is the author of object change for manualy triggered imports.

As of now every object that was changed by import had userModification set to 0 (that is system user).
This change introduce new column "userOwner" to queue table that by default is set to 0.
If user manualy trigger import it's user id is passed to queue item and later in import process it is set to element userModification property.
This will not affect automatic/cron imports.

Copy link

github-actions bot commented Jul 16, 2024

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@root913
Copy link
Contributor Author

root913 commented Jul 16, 2024

I have read the CLA Document and I hereby sign the CLA

Copy link
Contributor

@mattamon mattamon left a comment

Choose a reason for hiding this comment

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

Hey @root913,

your PR looks good to me! Just some minor changes. I would always rather import the whole class instead of importing just a part like with Tool\Admin.

Also I would add a comment saying that the 0 is system user just to have it there.

But other than that it looks solid!

Thank you for your contribution!

src/DataSource/Interpreter/AbstractInterpreter.php Outdated Show resolved Hide resolved
@mattamon mattamon self-assigned this Aug 16, 2024
@root913
Copy link
Contributor Author

root913 commented Aug 20, 2024

I noticed that latest migration is specified in Installer class (method: getLastMigrationVersionClassName) - should i update it also?

@root913 root913 requested a review from mattamon August 21, 2024 07:30
@mattamon
Copy link
Contributor

@root913

I noticed that latest migration is specified in Installer class (method: getLastMigrationVersionClassName) - should i update it also?

That would be awesome yeah!

@root913 root913 requested a review from mattamon August 22, 2024 16:54
@mattamon mattamon modified the milestones: 2.0.0, 1.10.0 Aug 26, 2024
Copy link
Contributor

@mattamon mattamon left a comment

Choose a reason for hiding this comment

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

@root913 Thank you for all the changes and your contribution! This looks good to me!

@mattamon mattamon merged commit 8ed5742 into pimcore:1.x Aug 26, 2024
8 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Aug 26, 2024
@root913 root913 deleted the feature/add-user-id branch August 26, 2024 06:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants