Fixes #10853 - add config_template_ids back to organization #2470
Conversation
|
This is breaking bats tests http://ci.theforeman.org/job/systest_foreman_hammer_nightly/133/default/ |
| @@ -76,6 +77,13 @@ def destroy | |||
|
|
|||
| private | |||
|
|
|||
| def rename_config_template | |||
| if params[taxonomy_single.to_sym] && !params[taxonomy_single.to_sym][:config_template_ids].nil? | |||
There was a problem hiding this comment.
Could you please modify params[taxonomy_single.to_sym] to params[taxonomy_single]? we don't need to convert it to symbol since params are HashWithIndifferentAccess. I know it's not related to this issue but I'd consider the same change everywhere in this controller as acceptable in this PR.
|
One more thing, we're missing :provisioning_template_ids in apipie param group. |
mbacovsky
force-pushed
the
10853_config_template_ids
branch
from
1373d7f
to
7a9413b
Compare
theforeman-bot
added
Needs re-review
Needs testing
and removed
Waiting on contributor
labels
|
@ares, .to_sym was removed and provisioning_template_ids was added into apipie doc. Note for testing: it propagates to hammer |
|
Except one nit it's OK. I created a hammer issue to provide |
mbacovsky
force-pushed
the
10853_config_template_ids
branch
from
7a9413b
to
b9965fe
Compare
theforeman-bot
added
Needs re-review
Needs testing
and removed
Waiting on contributor
labels
|
@ares, |
|
👍 pending tests |
|
[test] |
| @@ -20,6 +21,7 @@ module Api::V2::TaxonomiesController | |||
| param :compute_resource_ids, Array, N_("Compute resource IDs"), :required => false | |||
| param :media_ids, Array, N_("Media IDs"), :required => false | |||
| param :config_template_ids, Array, N_("Provisioning template IDs"), :required => false | |||
| param :provisioning_template_ids, Array, N_("Provisioning template IDs"), :required => false | |||
There was a problem hiding this comment.
Leave a note - deprecated or something like that
There was a problem hiding this comment.
I was against custom deprecated string here, it's planned to have support for marking params as deprecated in apipie, maybe leaving a comment to easily find this later would be enough
There was a problem hiding this comment.
@elobato I've added just a FIXME comment to locate the place. We should add support for deprecated options into Apipie first. Then this is information could be reused in docs and Hammer. I filed issue as a reminder http://projects.theforeman.org/issues/10869
mbacovsky
force-pushed
the
10853_config_template_ids
branch
from
b9965fe
to
ebceac9
Compare
|
@elobato anything that prevents this PR to be merged or can I go ahead? |
|
Merged as ebceac9, thanks @mbacovsky! |
before_filter converting
config_template_idstoprovisioning_template_idsis missing fromtaxonomieswhich is breaking backward compatibility and hammer bats tests