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

Code for Org Create orchestration #3743

Merged
merged 1 commit into from Mar 7, 2014
Merged

Conversation

parthaa
Copy link
Contributor

@parthaa parthaa commented Feb 26, 2014

No description provided.

end

include AsyncOrchestration
include Ext::PermissionTagCleanup

include Katello::Authorization::Organization
include Glue::ElasticSearch::Organization if Katello.config.use_elasticsearch
Copy link
Member

Choose a reason for hiding this comment

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

I think we still need to include the module, as there is the mapping defined. We just need to disable the autoreindex when creating it with dynflow (organization.disable_auto_reindex!)

@bkearney
Copy link
Member

please make sure to check the hammer org command that it still works with this.

@bkearney they will once -> https://github.com/iNecas/foreman-tasks/pull/35 gets merged and a new foreman-tasks gem gets rolled out by @iNecas .. .

content_view.disable_auto_reindex! if ::Katello.config.use_elasticsearch
content_view.save!
plan_action(ElasticSearch::Reindex, content_view) if ::Katello.config.use_elasticsearch
end
Copy link
Member

Choose a reason for hiding this comment

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

this indentation looks wonky

@jlsherrill
Copy link
Member

Tested and all seems well. Will ack once small formatting issues addressed.

'organization_label' => content_view.organization.label,
'cp_id' => content_view_environment.cp_id,
'name' => content_view_environment.label,
'description' => content_view.description)
Copy link
Member

Choose a reason for hiding this comment

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

i would think you'd want:

if Katello.config.use_cp
          plan_action(Candlepin::Environment::Create,
                      'organization_label' => content_view.organization.label,
                      'cp_id' => content_view_environment.cp_id,
                      'name' => content_view_environment.label,
                      'description' => content_view.description)
end

@iNecas
Copy link
Member

iNecas commented Mar 4, 2014

@parthaa do you plan to add the tests for the rest of added actions? I see only Organization create

@iNecas Added tests for the rest of them.. Need your help for the library_create_test. For some reasons the fixtures don't load for just that one test..
When I run parthaa@5dadeb5#diff-20 I get an error like https://gist.github.com/parthaa/9354826
Note the action in the gist seems to think its running Actions::Headpin::Environment::Create while its actually running ::Actions::Headpin::Environment::LibraryCreate .. Do I need to change this section ?

    describe "Create" do
       let(:action_class) { ::Actions::Headpin::Environment::LibraryCreate }
       let(:action) { create_action action_class }
      ....
    end

@parthaa yes: change "Create" to "LibraryCreate". the fixtures are not working probaby because thery are not loaded into the spec directly. I preffer FactoryGirl, as it allows to build object without actually saving it, which works usually better when testing actions, saving some unnecessary calls to db.

@iNecas weirdly enough I fixed this with this hack....
parthaa@a3e5839#diff-482caa01bfe25cd1cdb410483d83d83bR17

I changed ->

- describe ::Actions::Headpin::Environment do
+ describe ::Actions::Headpin::ContentView do

for some reason all the fixtures load fine the second I do this change..

@parthaa
Copy link
Contributor Author

parthaa commented Mar 5, 2014

@daviddavis can I ack myself on this :).......

@iNecas
Copy link
Member

iNecas commented Mar 6, 2014

@bkearney @parthaa the foreman-tasks waiting for action fixed and released in 0.3.5

@parthaa
Copy link
Contributor Author

parthaa commented Mar 6, 2014

[test]

@@ -24,7 +24,7 @@ class ContentViewVersion < Katello::Model
:class_name => "Katello::KTEnvironment",
:inverse_of => :content_view_versions,
:before_add => :add_environment,
:after_remove => :remove_environment
:after_remove => :remove_environment
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this intentional? It looks off.

@daviddavis
Copy link
Contributor

APJ

@daviddavis
Copy link
Contributor

[test]

@parthaa
Copy link
Contributor Author

parthaa commented Mar 7, 2014

sounds like a plan... Thanks @daviddavis and @iNecas !

daviddavis pushed a commit that referenced this pull request Mar 7, 2014
Code for Org  Create orchestration
@daviddavis daviddavis merged commit 0bda7f5 into Katello:master Mar 7, 2014
@parthaa parthaa deleted the org_create branch April 28, 2014 20:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants