search

pact-foundation / pact_broker

Enables your consumer driven contracts workflow
http://pactflow.io
MIT License
705 stars 173 forks source link

Adapt the Pact Broker UI to support removing pacts #179

Closed debae closed 5 years ago

debae commented 6 years ago

Currently the Pact Broker only has a REST endpoint where you can use the DELETE http method to remove contracts. The idea is to be able to do this through the UI as well. I created some screenshots what I had in mind.

Add an icon to remove the complete pact on the overview page : image

On the detail page of the pact add two links, one to remove only the specific version, and a link as well to remove the complete pact :

image

bethesque commented 6 years ago

Thanks for looking at this Baerten.

We're very limited for space on the index, especially once people start using git hashes and add the tags in using tags=true. Have a look at this PR. Given that deleting is something that is likely to be infrequent, I think the delete link would be best just on the pact page itself.

On the pact page, I'm hesitant to mix delete links amongst navigation links. It reminds me of the Jenkins menu which has "Delete" right under "Configure"!!! How about an "Actions..." link (or button?) on the pact page, which brings up a menu with "Delete this pact version" and "Delete all pact versions" in it? With a confirm once it has been clicked, just to be sure!

bethesque commented 6 years ago

On a side note, is the text not showing correctly in the badge, or is that just an artefact of the screenshot?

debae commented 6 years ago

Oh, I just removed the project specific information, the badge is working correctly. I'll only change the pact page then, and not the overview.

bethesque commented 6 years ago

Cool. Let me know what you've got in mind for separating the destructive links from the navigation links.

bethesque commented 6 years ago

How is this coming along?

JustAnotherSoftwareDeveloper commented 6 years ago

If nobody is doing this mind if I pick it up? My company has started to use pact and it would behoove me to have an easy way to remove these pacts, especially since many developers are unfamiliar with the pact api.

Granted it would take some time to get used to the project and coding style.

bethesque commented 6 years ago

That would be awesome. It should be super simple. If you're a js person, I'd suggest just a little js snippet to send the delete request, and then redirect to home page.

I see why you were interested in separating the UI now.

bethesque commented 6 years ago

In the meantime, the easiest way to delete a version of the pact this to click the "view in api browser" button and then send a delete request to the resource.

JustAnotherSoftwareDeveloper commented 6 years ago

I'll try to get to it Saturday. Note I'm pretty unfamiliar with the way you're constructing web pages so it might take a bit to get myself comfortable with the code base.

bethesque commented 6 years ago

That particular page is constructed in a bizarre kind of way. I already had the code to create a markdown representation of the pact. This was originally used in the Ruby Pact codebase. When the Pact Broker was written, I took that code, and then used another library to convert it to HTML. I then whacked a few custom HTML fields around it. This is why the HTML Pact Renderer class is so bizarre!

https://github.com/pact-foundation/pact_broker/blob/master/lib/pact_broker/api/renderers/html_pact_renderer.rb

Originally, I reused the pact -> markdown converter code by including the Ruby Pact library as a dependency. Later on, I wanted to break that dependency, so I copied the code into the Pact Broker codebase. You'll find it here, though you won't need to modify it.

My main concern with this piece of work is that adding an extra link to an already crowded section of the page is going to look pretty crap. I think we've reached the stage where we need an "More..." or "Actions..." UI element that reveals a menu with the "Delete" item in it. As I mentioned above, just adding a Delete hyperlink (a destructive action) intermingled with navigation hyperlinks is not good UI design. TBH, this is the reason why I haven't gotten around to this piece of work, because while I know what I want to achieve, I'm so bad at the actual element design and I'm out of touch with javascript/css because I've been working on APIs so long now!

image

Happy to bounce around some thoughts with you on design.

JustAnotherSoftwareDeveloper commented 6 years ago

We could do something similar to what boostrap 4 does with button groups: https://getbootstrap.com/docs/4.0/components/dropdowns/#menu-items

This would allow us to but a number of links into a "More..." menu.

My biggest concern is that I'm pretty unfamiliar with the server-rendered paradigm. I'm sure (with some time to get familiar with the stack) this feature could be accomplished, but I don't know if this method of development is conducive to a larger, more feature filled, UI.

If we're just going for a solution to this specific problem I don't think that's a major concern, but eventually the (already very robust) API is going to be limited by the minimal UI in terms of effective usage.

bethesque commented 6 years ago

Eventually, I expect the entire server side rendered UI to be replaced with a fully functioning JS UI. That's what we're already working on for the (paid for) version of the Pact Broker at https://pact.dius.com.au

I think that bootstrap button would be ok. Keep in mind it's an older version of bootstrap :(

bethesque commented 6 years ago

You shouldn't really have to worry about it being a server side render because that particular piece of code (if you look at the link I sent) is just concatenating the HTML together in a normal class.