Project

General

Profile

Bug #16463

with_taxonomy_scope returns all objects when no taxable ids available

Added by Daniel Lobato Garcia about 4 years ago. Updated over 2 years ago.

Status:
Closed
Priority:
High
Category:
Organizations and Locations
Target version:
Difficulty:
Triaged:
Bugzilla link:
Fixed in Releases:
Found in Releases:

Description

Currently scoping by taxonomies is broken.

Before this commit: https://github.com/theforeman/foreman/commit/35ed04f95f14c934ffebc874e90a3a4a6684fe71

`scope = scope.where(:id => taxable_ids) if taxable_ids`

did STILL call `scope.where(:id => [])` when taxable_ids were not found.
That returns an empty scope, and is the appropriate thing to do if there are no taxable IDs.
However after the commit, `if taxable_ids.present?` will make scope return `1=1` for the case where there are no taxable IDs.

If there are some taxable IDs, it does the right thing and scopes the object properly. That use case works fine.


Related issues

Related to Foreman - Bug #16389: Can't create taxable e.g. domain object if I'm in context of specific organization or locationClosed2016-08-31

Associated revisions

Revision d08290be (diff)
Added by Daniel Lobato Garcia about 4 years ago

Fixes #16463 - Fix with_taxonomy_scope when taxable_ids = []

The old implementation of taxonomy_scope was relying on calling
`scope.where(:id => [])`, it did it 'if taxable_ids'. A recent change
introduced more checking into that condition (taxable_ids.present?)
which made the case when taxable_ids == [] not run any scope.

That results in objects not being scoped at all when taxable_ids
is an empty array.

Revision b7765581 (diff)
Added by Daniel Lobato Garcia about 4 years ago

Fixes #16463 - Fix with_taxonomy_scope when taxable_ids = []

The old implementation of taxonomy_scope was relying on calling
`scope.where(:id => [])`, it did it 'if taxable_ids'. A recent change
introduced more checking into that condition (taxable_ids.present?)
which made the case when taxable_ids == [] not run any scope.

That results in objects not being scoped at all when taxable_ids
is an empty array.

(cherry picked from commit d08290be7b0bca2d4517e297b46daa41949f8791)

Revision 3c62e174 (diff)
Added by Daniel Lobato Garcia about 4 years ago

Fixes #16463 - Fix with_taxonomy_scope when taxable_ids = []

The old implementation of taxonomy_scope was relying on calling
`scope.where(:id => [])`, it did it 'if taxable_ids'. A recent change
introduced more checking into that condition (taxable_ids.present?)
which made the case when taxable_ids == [] not run any scope.

That results in objects not being scoped at all when taxable_ids
is an empty array.

(cherry picked from commit d08290be7b0bca2d4517e297b46daa41949f8791)

History

#1 Updated by Dominic Cleal about 4 years ago

  • Related to Bug #16389: Can't create taxable e.g. domain object if I'm in context of specific organization or location added

#2 Updated by Dominic Cleal about 4 years ago

  • Status changed from New to Assigned
  • Legacy Backlogs Release (now unused) set to 181

#3 Updated by Marek Hulán about 4 years ago

oh my... sorry

#4 Updated by The Foreman Bot about 4 years ago

  • Status changed from Assigned to Ready For Testing
  • Pull request https://github.com/theforeman/foreman/pull/3824 added

#5 Updated by Marek Hulán about 4 years ago

  • Legacy Backlogs Release (now unused) changed from 181 to 160

The introducing commit is not present in 1.12 so I think this should go to 1.13 where it's present. Feel free to reset if I'm mistaken.

#6 Updated by Dominic Cleal about 4 years ago

The change introducing the bug is currently set for inclusion in 1.12.3, so just setting this the same. Should #16389 be changed?

#7 Updated by Daniel Lobato Garcia about 4 years ago

At your will, both have to be introduced at the same time though. I'd prefer to introduce them both on 1.13 to play safe with 1.12.3

#8 Updated by Dominic Cleal about 4 years ago

Yes, they need to be on the same release. I opted for the point release on the other because I felt it was a somewhat severe bug, though it has a reasonably easy workaround.

#9 Updated by Marek Hulán about 4 years ago

  • Legacy Backlogs Release (now unused) changed from 160 to 181

Ok, makes sense, moving this back to 1.12.3 because of severity. Sorry for the noise.

#10 Updated by Daniel Lobato Garcia about 4 years ago

  • Status changed from Ready For Testing to Closed
  • % Done changed from 0 to 100

Also available in: Atom PDF