fixes #3912 - add inheritance for locations / organizations #1105

isratrade · 2013-12-18T16:27:56Z

No description provided.

isratrade · 2013-12-18T16:29:15Z

app/assets/stylesheets/application.scss

@@ -203,6 +203,7 @@ select{padding: initial;}
 .collapse .inputs-list li{ margin-left:20px; }

 .delete{color: #b94a48 !important; cursor: auto;}
+.inherited{font-weight: bold !important; cursor: auto;}


this is just to differentiate disabled items that are disabled because used by host or disabled because inherited. There is is a tooltip if disabled because inherited

isratrade · 2013-12-19T15:18:42Z

re-committed. added some tests and ready for review

ohadlevy · 2013-12-30T12:49:51Z

app/controllers/concerns/foreman/controller/taxonomies_controller.rb

@@ -35,6 +35,14 @@ def new
    end
  end

+  def nest
+    @taxonomy = taxonomy_class.new
+    Taxonomy.no_taxonomy_scope do


why without scope? shouldnt it be the parent scope or something like that? could this be abused?

isratrade · 2014-01-07T20:00:00Z

@ohadlevy, do you want this feature merged soon?

ohadlevy · 2014-01-07T21:00:41Z

@isratrade yes please :)

isratrade · 2014-01-08T12:10:14Z

@abenari, I have one failure caused by test/unit/classification_test.rb:38
It seems to have to do with the matcher but I didn't add inheritance in the fixtures, so I'm confused what changed.

<"test"> expected but was
<"secret">. (test_0004_enc_should_return_updated_cluster_param)
/var/lib/workspace/workspace/test_develop_pull_request@4/database/sqlite3/puppet/3.0/ruby/1.8.7/test/unit/classification_test.rb:38

ohadlevy · 2014-01-09T07:57:00Z

@isratrade ping?

isratrade · 2014-01-09T08:02:28Z

@ohadlevy, I don't understand the test failure to classification_test. It would be good if another set of eyes could look at it.

isratrade · 2014-01-09T09:36:26Z

@ohadlevy, classification_test now passes after some not changing to_label. Note that without PR #1104, the inherited items from parent (on the right selected side) can be unselected which is not good.

isratrade · 2014-01-09T12:16:05Z

re-committed and access_permission_test now passes. [test]

isratrade · 2014-01-13T12:23:06Z

same issue. failure only on mysql / ruby 2 that I don't think is relevant

isratrade · 2014-01-13T14:39:12Z

Added new commit to disable used locations/organizations on edit form for domain, subnet, etc. I also included the js needed in PR #1104

isratrade · 2014-01-14T08:32:32Z

re-committed with fixes to taxonomix.rb that caused failing tests

domcleal · 2014-01-14T13:26:53Z

@isratrade ignore the sqlite/1.9.3 error, but you have integration test failures

isratrade · 2014-01-14T16:27:43Z

@domcleal, not sure about the integration failures that popped up. I can't reproduce them locally. I'm continue tomorrow.

isratrade · 2014-01-15T12:28:18Z

app/controllers/application_controller.rb

@@ -17,6 +17,7 @@ class ApplicationController < ActionController::Base
  before_filter :set_taxonomy, :require_mail, :check_empty_taxonomy
  before_filter :welcome, :only => :index, :unless => :api_request?
  before_filter :authorize
+  before_filter :clean_taxonomy_ids, :only => [:create, :update]


this is needed, since the the locations / organizations tab was switch to use multi-select.js and apparently this js library sends duplicate ids
ex.
params[:domain][:location_ids] = [","45", "45"] even though the [selected_ids] array does not have any duplicates.

isratrade · 2014-01-19T15:21:09Z

there are still two test failures on cloning location / organization because Smart Proxy fixtures are not valid due to duplicate URL's

isratrade · 2014-01-20T15:46:56Z

app/models/host/managed.rb

-      conditions[:organization_id] = Array.wrap(org).map(&:id) if org
-      conditions[:location_id]     = Array.wrap(loc).map(&:id) if loc
+      conditions[:organization_id] = org.path_ids if org
+      conditions[:location_id]     = loc.path_ids if loc


all tests should now pass. I also made this change above and squashed it.

isratrade · 2014-01-22T13:13:11Z

added new commit. changes are.

context of viewing of hosts (or other objects), scope by location / organization and their children (not parents)
context of creating new host, scope objects by location / organization and their parents
[test]

domcleal · 2014-02-14T15:17:38Z

app/models/taxonomies/location.rb

+  end
+
+  def selected_or_inherited_parameters
+    LocationParameter.where(:reference_id => path_ids)


This isn't as intelligent as Hostgroup#parameters. The hg version will iterate over the ancestry in order and override parameters as it gets deeper, while this implementation won't - we should unify them.

@domcleal, to re-factor this efficiently, I either need to cherry-pick the changes to NestedAncestryCommon in PR ##1155 or wait until it's merged. Or, I can temporarily duplicate code from hostgroup.rb here in location.rb if this PR is going to be merged first.

I'll do 1155 first then, thanks.

domcleal · 2014-02-14T16:09:03Z

By the way, you have some integration test failures.

One more difference I noticed between this and host groups is that the parent host group is shown in a select box when you edit it. Could we add this to the taxonomy edit page? I think it'd be safe to make it editable even, but if there's an issue, then it could be left disabled.

isratrade · 2014-02-16T12:12:42Z

app/models/taxonomies/location.rb

      }

+  scoped_search :on => :label, :complete_value => :true, :default_order => true


this was added to location.rb and organization.rb even though it appears in the NestedAncestryCommon mix. Otherwise, the autocompleter won't find "label = "

isratrade · 2014-02-16T15:39:34Z

I assume the failure on Ruby 1.8.7 sqlite3 only is from jenkins.

domcleal · 2014-02-17T11:31:19Z

app/controllers/concerns/foreman/controller/taxonomies_controller.rb

@@ -87,6 +95,9 @@ def destroy
    else
      process_error
    end
+  rescue Ancestry::AncestryException


[done] Can you add this to the taxonomies API controller concern too please?

yes, added to api controller

isratrade · 2014-02-17T15:19:20Z

re-factored and with all fixes to comments.

isratrade · 2014-02-17T15:21:16Z

app/controllers/concerns/foreman/controller/taxonomies_controller.rb

@@ -70,8 +75,7 @@ def step2

  def update
    result = Taxonomy.no_taxonomy_scope do
-      (params[taxonomy_single.to_sym][:ignore_types] -= ["0"]) if params[taxonomy_single.to_sym][:ignore_types]


this is now handle in the before_validation in the model

isratrade · 2014-02-18T09:05:10Z

@domcleal, if I use the same code for inherited parameters in locs/orgs as in hostgroups, then it doesn't show inherited values. Should this bug http://projects.theforeman.org/issues/4348 need to prevent the merger of this PR?

domcleal · 2014-02-18T09:06:46Z

@isratrade no, let's fix that later, it's only a UI issue

isratrade · 2014-02-18T14:58:13Z

@ohadlevy, removed unused view partial and re-commited

isratrade · 2014-02-18T18:36:33Z

rebased after multi-select commit

domcleal · 2014-02-19T10:34:36Z

Thanks @isratrade! Merged as 1fa008a for Foreman 1.5. Nice work :)

isratrade reviewed Dec 18, 2013
View reviewed changes

ohadlevy reviewed Dec 30, 2013
View reviewed changes

isratrade reviewed Jan 15, 2014
View reviewed changes

isratrade reviewed Jan 20, 2014
View reviewed changes

dLobatog mentioned this pull request Jan 24, 2014

Feature #3914 Set taxonomies from user defaults on login #1191

Closed

domcleal reviewed Feb 14, 2014
View reviewed changes

domcleal added Waiting on contributor and removed Needs testing labels Feb 14, 2014

isratrade reviewed Feb 16, 2014
View reviewed changes

domcleal reviewed Feb 17, 2014
View reviewed changes

isratrade reviewed Feb 17, 2014
View reviewed changes

fixes theforeman#3912 - add inheritance for locations / organizations

2460ff5

domcleal added Ready to merge and removed Waiting on contributor labels Feb 19, 2014

domcleal closed this Feb 19, 2014

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fixes #3912 - add inheritance for locations / organizations #1105

fixes #3912 - add inheritance for locations / organizations #1105

isratrade commented Dec 18, 2013

isratrade Dec 18, 2013

isratrade commented Dec 19, 2013

ohadlevy Dec 30, 2013

isratrade commented Jan 7, 2014

ohadlevy commented Jan 7, 2014

isratrade commented Jan 8, 2014

ohadlevy commented Jan 9, 2014

isratrade commented Jan 9, 2014

isratrade commented Jan 9, 2014

isratrade commented Jan 9, 2014

isratrade commented Jan 13, 2014

isratrade commented Jan 13, 2014

isratrade commented Jan 14, 2014

domcleal commented Jan 14, 2014

isratrade commented Jan 14, 2014

isratrade Jan 15, 2014

isratrade commented Jan 19, 2014

isratrade Jan 20, 2014

isratrade commented Jan 22, 2014

domcleal Feb 14, 2014

isratrade Feb 16, 2014

domcleal Feb 17, 2014

domcleal commented Feb 14, 2014

isratrade Feb 16, 2014

isratrade commented Feb 16, 2014

domcleal Feb 17, 2014

isratrade Feb 17, 2014

isratrade commented Feb 17, 2014

isratrade Feb 17, 2014

isratrade commented Feb 18, 2014

domcleal commented Feb 18, 2014

isratrade commented Feb 18, 2014

isratrade commented Feb 18, 2014

domcleal commented Feb 19, 2014

		}

		scoped_search :on => :label, :complete_value => :true, :default_order => true

fixes #3912 - add inheritance for locations / organizations #1105

fixes #3912 - add inheritance for locations / organizations #1105

Conversation

isratrade commented Dec 18, 2013

Choose a reason for hiding this comment

isratrade commented Dec 19, 2013

Choose a reason for hiding this comment

isratrade commented Jan 7, 2014

ohadlevy commented Jan 7, 2014

isratrade commented Jan 8, 2014

ohadlevy commented Jan 9, 2014

isratrade commented Jan 9, 2014

isratrade commented Jan 9, 2014

isratrade commented Jan 9, 2014

isratrade commented Jan 13, 2014

isratrade commented Jan 13, 2014

isratrade commented Jan 14, 2014

domcleal commented Jan 14, 2014

isratrade commented Jan 14, 2014

Choose a reason for hiding this comment

isratrade commented Jan 19, 2014

Choose a reason for hiding this comment

isratrade commented Jan 22, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

domcleal commented Feb 14, 2014

Choose a reason for hiding this comment

isratrade commented Feb 16, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

isratrade commented Feb 17, 2014

Choose a reason for hiding this comment

isratrade commented Feb 18, 2014

domcleal commented Feb 18, 2014

isratrade commented Feb 18, 2014

isratrade commented Feb 18, 2014

domcleal commented Feb 19, 2014