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.

Sign up for GitHub

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

Jump to bottom

Add DELETE service_templates/X/schedules/X #414

Merged
merged 1 commit into from
Jul 11, 2018
Merged

Add DELETE service_templates/X/schedules/X #414

merged 1 commit into from
Jul 11, 2018

Conversation

bdunne
Copy link
Member

@bdunne bdunne commented Jul 9, 2018 •
edited
Loading

@abellotti Please review

Enables deleting schedules for ServiceTemplates for V2V
https://bugzilla.redhat.com/show_bug.cgi?id=1564255

@bdunne bdunne force-pushed the delete_schedules_for_service_templates branch from 3a2ece1 to 888aff5 Compare July 9, 2018 20:11
abellotti
abellotti reviewed Jul 9, 2018
View reviewed changes
app/controllers/api/service_templates_controller.rb Outdated
@@ -50,6 +50,10 @@ def schedules_query_resource(object)
object.miq_schedules
end

def delete_resource_schedules(_parent, type, id, data)
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 this method should go to app/controllers/api/subcollections/schedules.rb

Copy link
Member Author

Choose a reason for hiding this comment

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

I couldn't include that mixin here since the relation works differently for ServiceTemplates than it previously did.

config/api.yml
@@ -2518,9 +2518,12 @@
:identifier: miq_report_reports
:options:
- :subcollection
:verbs: *gp
:verbs: *gpd
:klass: MiqSchedule
:subcollection_actions:
Copy link
Member

Choose a reason for hiding this comment

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

maybe add the post equivalent (POST action "delete") for completeness. No additional coded needed just the added specs.

Copy link
Member

Choose a reason for hiding this comment

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

unless you wanted this PR for just the DELETE

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 done

abellotti
abellotti reviewed Jul 9, 2018
View reviewed changes
config/api.yml Outdated
:klass: MiqSchedule
:subcollection_actions:
:delete:
Copy link
Member

Choose a reason for hiding this comment

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

hmm, the delete should probably move to a subresource_actions section

@bdunne bdunne force-pushed the delete_schedules_for_service_templates branch from 888aff5 to 22fd104 Compare July 9, 2018 22:05
bdunne
bdunne commented Jul 9, 2018
View reviewed changes
config/api.yml
:klass: MiqSchedule
:subcollection_actions:
:get:
- :name: read
:identifier: miq_report_view
:post:
- :name: delete
:identifier: schedule_delete
Copy link
Member Author

Choose a reason for hiding this comment

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

Is this okay? ¯\_(ツ)_/¯

Copy link
Member

Choose a reason for hiding this comment

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

👍

@AllenBW AllenBW mentioned this pull request Jul 10, 2018
Merged
@bdunne bdunne force-pushed the delete_schedules_for_service_templates branch 4 times, most recently from a87b1b2 to 80269c0 Compare July 10, 2018 19:59
@bdunne bdunne force-pushed the delete_schedules_for_service_templates branch 2 times, most recently from 6675f01 to 86a65d6 Compare July 11, 2018 13:57
abellotti
abellotti reviewed Jul 11, 2018
View reviewed changes
spec/requests/service_templates_spec.rb Outdated
end

describe "POST /api/service_templates/:id/schedules/:id with delete action" do
it "can queue a flavor for deletion" do
Copy link
Member

Choose a reason for hiding this comment

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

can delete a schedule

@bdunne bdunne force-pushed the delete_schedules_for_service_templates branch from 86a65d6 to 1671344 Compare July 11, 2018 14:48
@miq-bot
Copy link
Member

miq-bot commented Jul 11, 2018

Checked commit bdunne@1671344 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🏆

@abellotti abellotti self-assigned this Jul 11, 2018
@abellotti
Copy link
Member

LGTM!! Thanks @bdunne for the API enhancement. 👍 will merge when 🍏

@abellotti abellotti added this to the Sprint 90 Ending Jul 16, 2018 milestone Jul 11, 2018
@abellotti abellotti merged commit b8d1480 into ManageIQ:master Jul 11, 2018
@bdunne bdunne deleted the delete_schedules_for_service_templates branch July 11, 2018 15:33
@simaishi
Copy link
Contributor

@bdunne Should this be gaprindashvili/yes and blocker?

@bdunne
Copy link
Member Author

bdunne commented Jul 25, 2018

@miq-bot add_labels blocker
@miq-bot add_labels gaprindashvili/yes

simaishi pushed a commit that referenced this pull request Jul 25, 2018
@abellotti @simaishi
Merge pull request #414 from bdunne/delete_schedules_for_service_temp…
489d291 
…lates

Add DELETE service_templates/X/schedules/X
(cherry picked from commit b8d1480)

https://bugzilla.redhat.com/show_bug.cgi?id=1608351
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit 489d291ffc1ed62e3dc62a234c5ed6b94a208915
Author: Alberto Bellotti <abellotti@users.noreply.github.com>
Date:   Wed Jul 11 11:21:50 2018 -0400

    Merge pull request #414 from bdunne/delete_schedules_for_service_templates
    
    Add DELETE service_templates/X/schedules/X
    (cherry picked from commit b8d14803f964e744a5f0446198991e6d6ffe8106)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1608351

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gtanzillo gtanzillo gtanzillo approved these changes

@abellotti abellotti abellotti approved these changes

Assignees

@abellotti abellotti

Labels
blocker enhancement gaprindashvili/backported
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@bdunne @miq-bot @abellotti @simaishi @gtanzillo