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 #23878 - Config association with organization #90

Merged
merged 1 commit into from Nov 22, 2018

Conversation

upadhyeammit
Copy link
Contributor

This adds missing dependent destroy of ForemanVirtWhoConfigure::Config class so when deleting the Organization it should consider deleting Virt Who configuration associated with Organization.

@upadhyeammit
Copy link
Contributor Author

@ares request you to review it once, I used discovery plugin as example when writing concern, I tested it and its working, I am able to delete org when having config defined but still you have look at this once.

Thank You!

@ares
Copy link
Member

ares commented Sep 26, 2018

Thanks @upadhyeammit, this seems to work with the original issue. There's still one more foreign key, that's failing though, I'm getting this now.

PG::ForeignKeyViolation: ERROR: update or delete on table "taxonomies" violates foreign key constraint "taxable_taxonomies_taxonomy_id_fk" on table "taxable_taxonomies" DETAIL: Key (id)=(4) is still referenced from table "taxable_taxonomies". : DELETE FROM "taxonomies" WHERE "taxonomies"."type" IN ('Organization') AND "taxonomies"."id" = $1

I'll need to look with my debugging environment, it's not clear why it fails now, perhaps it's related to the other linked objects from configuration, such as user

@upadhyeammit
Copy link
Contributor Author

@ares I will check this once, will let you know my findings.

@upadhyeammit
Copy link
Contributor Author

Hello,

If I see the error then I do see there are good number records in taxable_taxonomies referring to the organization which I tried to delete,

ESC[32m2018-10-22T07:28:11ESC[0m [ESC[31mEESC[0m|ESC[36mbacESC[0m|4fa15] PG::ForeignKeyViolation: ERROR: update or delete on table "taxonomies" violates foreign key constraint "taxable_taxonomies_taxonomy_id_fk" on table "taxable_taxonomies"
| DETAIL: Key (id)=(3) is still referenced from table "taxable_taxonomies".
| : DELETE FROM "taxonomies" WHERE "taxonomies"."id" = $1 (ActiveRecord::InvalidForeignKey)
| /home/vagrant/.rvm/gems/ruby-2.5.1/gems/activerecord-5.2.1/lib/active_record/connection_adapters/postgresql_adapter.rb:603:in async_exec' | /home/vagrant/.rvm/gems/ruby-2.5.1/gems/activerecord-5.2.1/lib/active_record/connection_adapters/postgresql_adapter.rb:603:in block (2 levels) in exec_no_cache'

katello=# select taxable_type, count(*) from taxable_taxonomies where taxonomy_id=3 group by taxable_type;
taxable_type | count
----------------------+-------
ProvisioningTemplate | 89
Ptable | 15
SmartProxy | 1
User | 1
Audited::Audit | 7
ReportTemplate | 4
(6 rows)

Foreign-key constraints:
"taxable_taxonomies_taxonomy_id_fk" FOREIGN KEY (taxonomy_id) REFERENCES taxonomies(id)

I feel like these associations should be getting handled somewhere already, I am checking it further, if you have any thoughts on this then request to comment.
_
Amit Upadhye.

@ares
Copy link
Member

ares commented Nov 22, 2018

@upadhyeammit that looks as the cause, you're right. The problem is that when we try to delete org, new audit record is created and therefore new entry in taxable_taxonomies table. This happens after these relation were cleanup by has_many :taxable_taxonomies :dependent => :destroy

That's definitely unrelated, so this can be merged. Thanks!

@ares ares merged commit 23c5a30 into theforeman:master Nov 22, 2018
@ares
Copy link
Member

ares commented Nov 22, 2018

so in fact, it was actually audit created by the virt who configuration deletion, not yet sure how to fix it

@ares
Copy link
Member

ares commented Nov 22, 2018

fix by the linked PR

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.

None yet

2 participants