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

Allow managers to delete API Backends that they manage #616

Closed
12 tasks done
brylie opened this issue Nov 11, 2015 · 30 comments
Closed
12 tasks done

Allow managers to delete API Backends that they manage #616

brylie opened this issue Nov 11, 2015 · 30 comments

Comments

@brylie
Copy link
Contributor

brylie commented Nov 11, 2015

User story

As a manager
I would like to delete an API Backend that I manage
so that I can keep my API Backends list up to date

Feature

Add a method for API Managers to delete API Backends they manage. The deletion should also remove the API Backend from API Umbrella.

Wireframes

User Managed APIs

My Managed APIs wireframe

View API Backend

View API Backend wireframe

Definition of done

  • Delete button exists in My managed APIs.
  • Delete button exists in View Api backend, if the user has the privilege to delete the API (is the owner/manager of that API or is an Apinf administrator).
  • When clicking delete, user is prompted for confirmation, i.e. making sure that they have not accidentally pushed that button.
    • Confirmation includes the name of the API backend to be removed.
  • After confirming, the API backend is deleted from Apinf API backends collection.
  • The related documents are also removed from respective collections:
    • API backlog
    • API feedback
    • API metadata
    • API documentation
  • API backend is deleted from API Umbrella via REST call to Admin API.
  • Only authorized users. i.e. API owners and Apinf administrators, can delete API backends.
@kyyberi
Copy link

kyyberi commented Nov 11, 2015

Highly needed :) Should have safe mechanism. Perhaps something like when deleting repository from Github.

@brylie
Copy link
Contributor Author

brylie commented Nov 12, 2015

Right, like the 'Danger Zone' box.

highway to the danger zone

@bajiat
Copy link
Contributor

bajiat commented Mar 3, 2016

@ashakunt and @Alapan Definition of done has been added. Wireframe to be added later today. Assigning task to Alapan.

@jykae
Copy link
Contributor

jykae commented Mar 3, 2016

Great, long waited feature for admin also 👍

@brylie
Copy link
Contributor Author

brylie commented Mar 3, 2016

@Alapan, we may already have code related to API Backend administrator permissions that would be useful here. Specifically, every API Backend document has a currentUserCanEdit collection helper that checks user permissions described in the definition of done.

@bajiat
Copy link
Contributor

bajiat commented Mar 7, 2016

@Alapan Can you comment whether this is in progress and what would be the estimation for completion? This week?

@Alapan
Copy link
Contributor

Alapan commented Mar 7, 2016

@bajiat Yes, this issue is in progress and I have implemented most of the front-end. It should be completed by this week.

@Alapan
Copy link
Contributor

Alapan commented Mar 8, 2016

@brylie I wanted to know if the REST endpoint for API backend deletion has been implemented? I was looking through https://github.com/apinf/meteor-api-umbrella/blob/master/server/apiUmbrellaWeb.js, where it seems the REST interfaces are defined, and couldn't find anything related to deletion.

@brylie
Copy link
Contributor Author

brylie commented Mar 8, 2016

Right, go ahead and add the deleteApiBackend method.

@Alapan
Copy link
Contributor

Alapan commented Mar 8, 2016

The deleteApiBackend method was implemented. Despite this, I am currently getting an error "Object [object Object] has no method 'deleteApiBackend'" when making the REST call apiUmbrellaWeb.adminApi.v1.apiBackends.deleteApiBackend(backendId) in apiBackends.js.

@brylie
Copy link
Contributor Author

brylie commented Mar 9, 2016

Make sure to update the package version and publish it to Atmosphere. Then, run meteor update in our project, so that the latest version of the package will be available. We may need to figure out how to publish a Meteor package as an organization, rather than an individual.

@sebbel
Copy link
Contributor

sebbel commented Mar 9, 2016

http://info.meteor.com/blog/meteor-091-organizations-blaze-apis

  1. Create apinf Account on meteor
  2. Create Organization on apinf account
  3. add members to organization

Am Mittwoch, 9. März 2016 schrieb Brylie Christopher Oxley :

Make sure to update the package version and publish it to Atmosphere.
Then, run meteor update in our project, so that the latest version of the
package will be available. We may need to figure out how to publish a
Meteor package as an organization, rather than an individual.


Reply to this email directly or view it on GitHub
https://github.com/apinf/api-umbrella-dashboard/issues/616#issuecomment-194178572
.

@brylie
Copy link
Contributor Author

brylie commented Mar 9, 2016

@sebbel what is your Meteor.com username?

@Alapan
Copy link
Contributor

Alapan commented Mar 9, 2016

@brylie I had to make another small change to the deleteApiBackend method in API umbrella - I had written HTTP.delete instead of HTTP.del - like PUT, POST and GET, I thought the function name is the same as the action. I fixed it and created a pull request (apinf/meteor-api-umbrella#17).

@Alapan
Copy link
Contributor

Alapan commented Mar 10, 2016

First working version created. Created a pull request.

@Alapan Alapan mentioned this issue Mar 10, 2016
@sebbel
Copy link
Contributor

sebbel commented Mar 12, 2016

@brylie my meteor account is sebbel

@Alapan
Copy link
Contributor

Alapan commented Mar 16, 2016

@brylie I resolved the conflicts in the Issue#616 branch and pushed it.

@brylie
Copy link
Contributor Author

brylie commented Mar 16, 2016

@Alapan see my line comment for client/views/api_backends/delete/deleteApiBackendConfirmation.js on line 54. The sAlert call is not in the if, else if, or else block. It looks like it is a logical error.

@saralavanip
Copy link
Contributor

I have tested 'delete' feature on my local machine.
Findings:

  • As a manager select API backend from 'Catalogue' and delete API backend returns to 'Catalogue' view.
  • If user has only one API backend added and delete it from 'Manage API backend' view, it stays on same page 'My Managed APIs' view.

@as33ms
Copy link
Contributor

as33ms commented Mar 16, 2016

thanks, @saralavanip

@as33ms
Copy link
Contributor

as33ms commented Mar 16, 2016

@Alapan did you check @brylie 's latest comment.

@saralavanip
Copy link
Contributor

Findings:

  • Add API backend and delete the same, returns to catalog view.

@saralavanip
Copy link
Contributor

Tested on local host.
Findings:

  • As a manager select API backend from 'Catalogue' and delete API backend returns to 'Catalogue' view
    and confirmation dialog not shown after successful delete API backend.
    .

@brylie
Copy link
Contributor Author

brylie commented Mar 17, 2016

@saralavanip will you please verify with @Alapan that your previous observation is resolved?

@jykae
Copy link
Contributor

jykae commented Mar 18, 2016

Noticed on review:
API Umbrella changes are made to the proxy configurations only after the changes have been published. This includes adding, updating, deleting. @brylie can probably point out what has to done to publish the changes for removal, as he fixed the publishing issue on adding API.

@saralavanip
Copy link
Contributor

@brylie , verified the resolved issue: 'confirmation dialog is shown after successful delete API backend from Catalog view'.

@saralavanip
Copy link
Contributor

@brylie , @bajiat , Deleting an API backend when it is down for some reason is unsuccessful. Could you please confirm if this is the expected result?

Screenshot:
api-backend-down-delete-unsuccessful

@brylie
Copy link
Contributor Author

brylie commented Mar 18, 2016

I am not sure about the cause of that error, but it should still be possible to delete the API Backend. Please file a bug report, so that we can troubleshoot.

@saralavanip
Copy link
Contributor

@brylie , please find issue reported https://github.com/apinf/api-umbrella-dashboard/issues/919.

@brylie
Copy link
Contributor Author

brylie commented Mar 21, 2016

@Alapan, I have tested this feature. It seems ready for merging, and we can work on enhancements/bugs in upcoming sprints.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants