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

api v2 - systems actions #3514

Merged
merged 1 commit into from
Jan 7, 2014
Merged

api v2 - systems actions #3514

merged 1 commit into from
Jan 7, 2014

Conversation

komidore64
Copy link
Contributor

  • - create
  • - index
  • - show
  • - update
  • - destroy
  • - tasks
  • - add_system_groups
  • - remove_system_groups
  • - package_profile
  • - report
  • - errata
  • - pools
  • - releases
  • - activate
  • - enabled_repos
  • - refresh_subscriptions
  • - tests!!
  • - squash commits

api :POST, "/systems", "Register a system"
param_group :system
api :POST, "/environments/:environment_id/consumers", "Register a system in environment (compatibility)"
Copy link
Member

Choose a reason for hiding this comment

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

What do you think about leaving this one out so it isn't in docs nor available through apipie?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@thomasmckay have the compatibility routes no-doc'd? i'd be cool with that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removing the rhsm compatibility routes doco in a different PR

@thomasmckay
Copy link
Member

Missing param_rules?

@thomasmckay
Copy link
Member

Ideally the v1 controller would be empty with all logic in v2.

@komidore64
Copy link
Contributor Author

@thomasmckay well of course, ideally

@thomasmckay
Copy link
Member

I had to edit param_rules to include search params (could be bad merge on my local pull-request branch?)

:index => [:page, :per_page, :sort_order, :sort_by, :paged, :system, :name, :pool_id, :search, :uuid, :organization_id, :environment_id, :system_group_id],

Also got an error on line 332

desc << "A hint for choosing the right value for the releaseVer param"

@daviddavis
Copy link
Contributor

Please squash these commits before you merge so we don't end up with another mess in our git history.

@thomasmckay
Copy link
Member

@komidore64

  • setup_content_view() missing from v2 controller (need to copy from v1)
  • i think :id should be in the update() param_rules, right?
  • line 585 is trying to add build something incorrectly
  NoMethodError: undefined method `+' for #<ActiveSupport::HashWithIndifferentAccess:0x007fc94d618c58>

@komidore64
Copy link
Contributor Author

@thomasmckay

  1. oops, will copy and add
  2. yes! i did miss it. i've got it fixed locally

@komidore64
Copy link
Contributor Author

@daviddavis will do

def param_rules
{
:create => [:system, :facts, :installed_products, :name, :type, :cp_type, :service_level, :location, :content_view_id, :environment_id, :organization_id],
:index => [:system, :name, :pool_id, :search, :uuid, :organization_id, :environment_id, :system_group_id],
Copy link
Member

Choose a reason for hiding this comment

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

Still missing :paged, :page_size, :page, and :sort. Probably testable in the UI.

@thomasmckay
Copy link
Member

ack pending jenkins

komidore64 added a commit that referenced this pull request Jan 7, 2014
@komidore64 komidore64 merged commit 527ad9c into Katello:master Jan 7, 2014
@komidore64 komidore64 deleted the systems-cont branch January 7, 2014 03:27
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.

3 participants