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

Refactor auditSync, auditDetach, auditAttach methods #866

Merged
merged 1 commit into from
Oct 10, 2023

Conversation

erikn69
Copy link
Contributor

@erikn69 erikn69 commented Oct 4, 2023

This PR keeps the current functionality, just better readability, PhpDoc fixes

@MortenDHansen Hi,
I want to make a change in the auditing of these relationship methods, I added Collection::diff o my local copy, so that only the added/removed records in the pivot are audited, and in the case that there are no changes and 'audit.empty_values' is false the audit would not be recorded
That change depends on this being accepted.

@erikn69 erikn69 closed this Oct 4, 2023
@erikn69 erikn69 reopened this Oct 4, 2023
@erikn69 erikn69 force-pushed the patch-18 branch 3 times, most recently from 41d9691 to 0a80c2f Compare October 4, 2023 21:28
@erikn69 erikn69 changed the title Avoid empty array when there is no data change on auditSync Refactor auditSync, auditDetach, auditAttach methods Oct 4, 2023
@erikn69 erikn69 requested a review from MortenDHansen October 4, 2023 21:39
@gisostallenberg
Copy link
Contributor

@erikn69 Could you look into #869 to see what can be improved there?

@MortenDHansen
Copy link
Contributor

Looks about right. :)

@erikn69
Copy link
Contributor Author

erikn69 commented Oct 10, 2023

Looks about right. :)

Ok, I will wait for it to be merged to add another PR and discuss the new functionality, I don't like refactoring something and adding features at the same time

@MortenDHansen MortenDHansen merged commit 7adc295 into owen-it:master Oct 10, 2023
11 checks passed
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.

4 participants