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

Fixes #29433 - seeding with scope #7597

Merged
merged 1 commit into from Apr 27, 2020

Conversation

ezr-ondrej
Copy link
Member

the seeding filter is broken if there is unexpected change to the checked resource.
Update audit contains only changed attributes

@theforeman-bot
Copy link
Member

Issues: #29433

@ezr-ondrej
Copy link
Member Author

[test foreman] moody integrational tests

Copy link
Member

@kgaikwad kgaikwad left a comment

Choose a reason for hiding this comment

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

Thank you @ezr-ondrej
LGTM +1

@kgaikwad
Copy link
Member

@tbrisker,
I request you to give second round of review. If no additional comments then I think good to go!

@kgaikwad
Copy link
Member

Works perfectly!

Tested the below scenarios:

  1. By renaming an existing default bookmark, db:seed runs smoothly without an error and won't create new record for the same.
  2. Renamed default bookmark (let's say with name error) to changed error and deleted this bookmark. Not created any new record for this default bookmark on db:seed.
  3. After step(2) deleted an associated audit records just for testing purpose, this time it created new record for deleted default bookmark on db:seed.

Copy link
Member

@tbrisker tbrisker left a comment

Choose a reason for hiding this comment

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

seeing the thorough testing i'll approve, but tbh I think we should stop relying on audits to decide what should be seeded. definitely out of scope for this pr but i'd love to get rid of all these audit checks in the seed helper, i think they cause more trouble than they solve.
Thanks @ezr-ondrej and @kgaikwad !

@tbrisker tbrisker merged commit fe4676d into theforeman:develop Apr 27, 2020
@ezr-ondrej ezr-ondrej deleted the seed_change_eval branch May 6, 2020 11:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants