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

Add delete endpoint for portfolio_item #84

Merged
merged 1 commit into from
Jan 7, 2019

Conversation

syncrou
Copy link
Contributor

@syncrou syncrou commented Jan 7, 2019

No description provided.

@syncrou syncrou added the enhancement New feature or request label Jan 7, 2019
@syncrou syncrou requested a review from mkanoor January 7, 2019 16:42
@syncrou syncrou mentioned this pull request Jan 7, 2019
@syncrou
Copy link
Contributor Author

syncrou commented Jan 7, 2019

cc - @lgalis

delete "/api/v0.0/portfolio_items/#{portfolio_item_id}", :headers => admin_headers, :params => valid_attributes
end

it 'deletes the record' do
Copy link
Contributor

Choose a reason for hiding this comment

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

@syncrou Do we need a TODO to add a test as a non admin user it should fail. Since we don't have any constraints check now we would have to wait. Also a GitHub issue might help

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a TODO that points to this Issue: #85

parameters:
- $ref: '#/parameters/PortfolioItemID'
responses:
204:
Copy link
Contributor

@mkanoor mkanoor Jan 7, 2019

Choose a reason for hiding this comment

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

@syncrou The 204 status reads

The HTTP 204 No Content success status response code indicates that the request has succeeded, but that the client doesn't need to go away from its current page

@lgalis Is the UI going to react properly for a 204, it would have to issue a refresh of the portfolio items page

@syncrou syncrou force-pushed the only_delete_portfolio_items branch from 51e8289 to 1786516 Compare January 7, 2019 17:18
@lgalis
Copy link
Contributor

lgalis commented Jan 7, 2019

@syncrou - looks good

@mkanoor mkanoor merged commit 8099175 into RedHatInsights:master Jan 7, 2019
gmcculloug pushed a commit to gmcculloug/catalog-api that referenced this pull request Nov 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants