gchq / gaffer-tools

gaffer-tools is deprecated. Use https://github.com/gchq/gafferpy instead
Apache License 2.0
50 stars 29 forks source link

Gh 791 add named operation group label & black/whitelist #830

Closed macenturalxl1 closed 4 years ago

macenturalxl1 commented 4 years ago

Hi - here is the UI feature to add a group name label to a NamedOperation and view it as a dynamically added prefix in the operation dropdown for Issue #791

Let me know what you think of the design how to view grouped NamedOperations in the dropdown (I've attached a screenshot on the Issue what it looks like from this implementation). I wasn't sure how far to go with the design - so kept it simple and in a state that is easy to change, maybe at a later date as this new feature matures.

*I'll add the whitelist/blacklist in another Pull Request to make continous integration easier - as this feature is something ready to use by a User. Or if you prefer I can put it all in a bigger Pull Request.

macenturalxl1 commented 4 years ago

Hi - I've made the change to add multiple label tags to a NamedOperation. Here is the design to add multiple label tags to a NamedOperation:

Screenshot 2020-04-23 at 13 22 13

It's a simple working design to add labels and is user friendly - I had a few technical limitations trying to develop something a bit more complex trying to pass in functions from the controller to the sidenav.html so this was my solution- let me know feedback on any word changing or a design change?

Also here is how label tags can be viewed in dropdown as:

Screenshot 2020-04-23 at 14 17 02 Screenshot 2020-04-23 at 14 22 36

I've added a feature to search by label tag, so by entering just '#' will bring back all NamedOperations with a label tag, and then entering the #tagname will bring back only NameOperations with that tag - so it can be grouped in some way ...

Also looks like there might be a failing test in error.service.spec.ts in the 'analytics-ui' module, but I think it still passes the stage. There is a second error in the module for custom-theme.scss and I think is causing the build to fail.

I've pulled the latest changes from develop branch and the 'ui' module I made my changes to builds successfully.

macenturalxl1 commented 4 years ago

Hi - just done some further investigation - in analytics-ui module at error.service.spec.ts:45:17, a new Error() is instanitated that throws an error but does not break the build or fail tests.

Here's the cause:

The pipeline is using Node v14.0.0, and Sass v4.13.1 only supports Node up to v13.x.x - error from stacktrace:

Error: Node Sass does not yet support your current environment: Linux 64-bit with Unsupported runtime (83)

Release notes. https://github.com/sass/node-sass/releases/tag/v4.13.1 showing Sass v4.13.1 only supports Node up to v13.x.x

Node has just released the latest v14.0.0 on 22/04/20 4am. Sass have just released a new version v4.14.0 also 22/04/20 after the release - hence why it has only just failed the builds yesterday.

Solution?:

It's recommended the pipeline environment should use the stable version of Node v12.16.2 to stop this happening again? (if the latest Node v14.0.0 features aren't important or aren't being used)

t11947 commented 4 years ago

Hi @macenturalxl1, thanks for the update on your progress. How might things be affected if a hash were to be included in the name of a label (or named operation for that matter)?

Thanks for bringing that node issue to our attention, we'll get that fixed as a priority.

macenturalxl1 commented 4 years ago

Hi @macenturalxl1, thanks for the update on your progress. How might things be affected if a hash were to be included in the name of a label (or named operation for that matter)?

Hi, the hashtags is just UI visual presentation - the HTML views append a # to the beginning of the label to view in the DOM - in the controller/model the labels are stored exactly as the User inputs them - including when they are added to the AddNamedOperation request body.

You make a good point as it could be confusing for the User thinking they're added to the NamedOperation with a # suffix if they are concerned with how it is implemented and if they don't see the request being made from the network tab in debug - unless for the User is just a black box User and want to any some form of label tagging?

Let me know if this requires a redesign

(I've also added a question on this Issue ticket in regards to whitelist/blacklist scenarios)

macenturalxl1 commented 4 years ago

Hi - any thoughts on the whiteList/blackList scenarios? > https://github.com/gchq/gaffer-tools/issues/791

I'll raise the white/blackList change in a separate PR.

macenturalxl1 commented 4 years ago

Hi I've fixed the pipeline where the version of Node was causing a dependency in Analytics to break.

I've also added the blacklist/whitelist feature to this using the rules confirmed in the Issue comments, let me know what you think > https://github.com/gchq/gaffer-tools/issues/791

macenturalxl1 commented 4 years ago

Hi - any feedback on this pull request?