Skip to content

Hide enable workflow button when group owner does not match workflow group matching regex #8699

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

Merged

Conversation

tylerjmchugh
Copy link
Contributor

Currently if a group is excluded from workflow the enable workflow button is still shown for records owned by that group. If the group is excluded from workflow the enable workflow button should be hidden.

This PR aims to fix this issue by hiding the enable workflow button when the group does not match the workflow group matching regex.

image
image
image

The workflow status label should also be hidden in the editor board for records owned by that group.

This PR fixes this issue by implementing logic to only show the status label when the record's group matches the workflow group matching regex.

image
image

Additionally, there is an issue where the workflow label is still shown in the metadata actions menu even when there are no workflow menu items.

This PR fixes this issue by showing the workflow label only when there is at at least one workflow menu item shown.

image image

Checklist

  • I have read the contribution guidelines
  • Pull request provided for main branch, backports managed with label
  • Good housekeeping of code, cleaning up comments, tests, and documentation
  • Clean commit history broken into understandable chucks, avoiding big commits with hundreds of files, cautious of reformatting and whitespace changes
  • Clean commit messages, longer verbose messages are encouraged
  • API Changes are identified in commit messages
  • Testing provided for features or enhancements using automatic tests
  • User documentation provided for new features or enhancements in manual
  • Build documentation provided for development instructions in README.md files
  • Library management using pom.xml dependency management. Update build documentation with intended library use and library tutorials or documentation

@tylerjmchugh tylerjmchugh marked this pull request as ready for review March 19, 2025 18:42
@ianwallen
Copy link
Contributor

It seems like you have hidden the buttons from the users but I'm guessing that the api's can still enable the workflow?

Do we also need to update api that are enabling the workflow to ensure that a workflow cannot be enabled on metadata where the group is not in the regexp?

@josegar74
Copy link
Member

josegar74 commented Mar 20, 2025

@tylerjmchugh testing the pull request I found that the Publish option from the metadata page for metadata in groups without the workflow enabled, shows this confirmation dialog:

When publishing records with workflow enabled, the status will change to 'Approve'. Are you sure you want to continue?

It can be good to update this also.


Another issue I found with metadata that was created in the Sample group before the workflow is enabled, with these changes the button to Enable workflow is not displayed, even if Sample group has the workflow enabled. Creating new metadata in Sample group, it works fine assigning it to the workflow.

@tylerjmchugh
Copy link
Contributor Author

@ianwallen I updated the API to throw an exception if attempting to set the status from null to any other status.

The reason I am only preventing going from null is to prevent the status from getting stuck. If a record already has workflow enabled before workflow is disabled for the owner group then the workflow status would be stuck in whatever state it was before disabling workflow for that group.

This way if a record is already submitted for example, the status can still be changed to approved if required.

@tylerjmchugh
Copy link
Contributor Author

@josegar74

@tylerjmchugh testing the pull request I found that the Publish option from the metadata page for metadata in groups without the workflow enabled, shows this confirmation dialog:

When publishing records with workflow enabled, the status will change to 'Approve'. Are you sure you want to continue?

It can be good to update this also.

I have updated the PR to include logic to only display this message if the record has workflow enabled.

Another issue I found with metadata that was created in the Sample group before the workflow is enabled, with these changes the button to Enable workflow is not displayed, even if Sample group has the workflow enabled. Creating new metadata in Sample group, it works fine assigning it to the workflow.

I am not able to reproduce this issue. Maybe I am misunderstanding.

I tried the following:

  • Disable workflow globally
    image
  • Create group named sample
    image
  • Create a record in sample
  • Workflow is disabled and the enable workflow button is hidden
    image
  • Enable workflow globally
    image
  • Workflow is still disabled but the enable workflow button is shown
    image
  • Enable workflow for all groups except sample
    image
  • Workflow is disabled and the enable workflow button is hidden
    image
  • Enable workflow for sample only
    image
  • Workflow is still disabled but the enable workflow button is shown
    image

I also tried:

  • Create group named sample
    image
  • Disable workflow for sample
    image
  • Create a record in sample
  • Workflow is disabled and the enable workflow button is hidden
    image
  • Enable workflow for sample
    image
  • Workflow is still disabled but the enable workflow button is shown
    image

The behaviour seems correct to me.

@ianwallen ianwallen requested a review from josegar74 March 26, 2025 20:26
@ianwallen ianwallen added this to the 4.4.7 milestone Mar 27, 2025
@josegar74 josegar74 modified the milestones: 4.4.7, 4.4.8 Apr 10, 2025
Copy link
Member

@josegar74 josegar74 left a comment

Choose a reason for hiding this comment

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

I have tested the changes and look good, but the code includes quite some Javascript ES6. Please check the comments to update it to ES5.

@@ -190,6 +190,8 @@ exception.resourceInvalid.metadata=Les fiches ne sont pas valides
exception.resourceInvalid.metadata.description=Les fiches ne peuvent pas \u00EAtre soumises ou approuv\u00E9es
exception.resourceNotEnabled.workflow=Le workflow est d\u00E9sactiv\u00E9
exception.resourceNotEnabled.workflow.description=Impossible de d\u00E9finir l'\u00E9tat des fiches
exception.resourceNotEnabled.groupWorkflow=Le workflow de groupe ''{0}'' est d\u00E9sactiv\u00E9
exception.resourceNotEnabled.groupWorkflow.description=Impossible de d\u00E9finir l'\u00E9tat des fiches dans le groupe ''{0}''
Copy link
Member

Choose a reason for hiding this comment

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

Can you rebase the pull request and update Messages_dut.properties, including for now the English values?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in latest push

* @returns {boolean} - True if the group name matches the workflow group matching regex, false otherwise.
*/
this.isGroupWithWorkflowEnabled = function (groupName) {
const workflowGroupMatchingRegex = gnConfig["metadata.workflow.draftWhenInGroup"];
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const workflowGroupMatchingRegex = gnConfig["metadata.workflow.draftWhenInGroup"];
var workflowGroupMatchingRegex = gnConfig["metadata.workflow.draftWhenInGroup"];

Remove ES6 syntax.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in latest push

// Check if there are any records
if (rec && rec.length) {
// Extract unique group owner IDs from the records
const groupIds = [...new Set(rec.map((md) => md.groupOwner))];
Copy link
Member

Choose a reason for hiding this comment

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

Please change to ES5 code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in latest push

Comment on lines 131 to 135
groupIds.forEach((groupId) => {
gnMetadataActions.getGroupName(groupId).then((groupName) => {
scope.groupNames[groupId] = groupName;
});
});
Copy link
Member

Choose a reason for hiding this comment

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

Please change to ES5 code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in latest push

var allowedProfiles = $scope.profiles.slice(
$scope.profiles.indexOf(profile)
);
return allowedProfiles.some((p) =>
Copy link
Member

Choose a reason for hiding this comment

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

Please change to ES5 code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in latest push

}
return scope
.getStatusEffects(user)
.some((step) => scope.displayWorkflowStepOption(step, user));
Copy link
Member

Choose a reason for hiding this comment

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

Please change to ES5 code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in latest push

@tylerjmchugh tylerjmchugh force-pushed the main.hide-enable-workflow-button branch from 6edf540 to 56e4a1d Compare May 16, 2025 12:12
@tylerjmchugh tylerjmchugh force-pushed the main.hide-enable-workflow-button branch from 06010a7 to 7e9425c Compare May 16, 2025 12:34
@josegar74 josegar74 modified the milestones: 4.4.8, 4.4.9 May 28, 2025
@ianwallen ianwallen merged commit cf79995 into geonetwork:main May 28, 2025
7 checks passed
@geonetworkbuild
Copy link
Collaborator

The backport to 4.2.x failed:

The process '/usr/bin/git' failed with exit code 1
stderr
error: could not apply ebffd054bf... Hide enable workflow button if group name does not match regex
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
hint: Disable this message with "git config set advice.mergeConflict false"

stdout
Auto-merging web-ui/src/main/resources/catalog/js/CatController.js
[backport-8699-to-4.2.x 9a42f75a05] add isProfileOrMoreForGroup method
 Author: tylerjmchugh <[email protected]>
 Date: Mon Mar 17 10:22:02 2025 -0400
 1 file changed, 11 insertions(+)
Auto-merging web-ui/src/main/resources/catalog/views/default/directives/directive.js
CONFLICT (content): Merge conflict in web-ui/src/main/resources/catalog/views/default/directives/directive.js

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-4.2.x 4.2.x
# Navigate to the new working tree
cd .worktrees/backport-4.2.x
# Create a new branch
git switch --create backport-8699-to-4.2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick 56db0006d6bec9d4220833a525118f0becb2e63d,ebffd054bfd0f464fa03b6a73de35c35a820bdc0,cc6446824453b36f2afe22356214dc30e2af6150,17e75ff0c380c7467dd6baac494a7e5db0e85340,da9361593a092aca87c0d365215e4eaca114c4d9,af746cec8d4793a901325d70d76e178973f3e281,28108421f2f21ed97ab6b2550dabffcef72b6b5f,8c489570f31ef3f24124c4bf4288564d2e3612c9,5e921ca9e8fa8ae81d8f01cf3d9ba24a4e3c64b0,ce3252b2baec2d8fdeaae5e709350e8e48b95ce4,59019e6fa7dd41ca3b48a8714ef5ab4d2c95efc4,56e4a1d66fd29135170115896fed23b1084f8657,6e0f00fb63cdd009a42435a3ff3fd518f7a4ff1f,7e9425cdd754b0f26d6204924182263bcd315cdb
# Push it to GitHub
git push --set-upstream origin backport-8699-to-4.2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-4.2.x

Then, create a pull request where the base branch is 4.2.x and the compare/head branch is backport-8699-to-4.2.x.

ianwallen pushed a commit to ianwallen/core-geonetwork that referenced this pull request May 28, 2025
…t match workflow group matching regex (geonetwork#8699)

* add isProfileOrMoreForGroup method

* Hide enable workflow button if group name does not match regex

* Move workflow option display logic to function

* Hide workflow label when no options are shown

* Move configuration variables to CatController

* Hide workflow status label from editor board

* Prevent enabling workflow for groups with workflow disabled in API

* Refactor

* Hide warnPublishDraft confirmation when workflow is not enabled

* Add message

* Display status label if workflow is already enabled

* Prevent setting status from null to any status for records owned by groups with workflow disabled

* Add dutch messages

* Remove ES6 code
ianwallen pushed a commit to ianwallen/core-geonetwork that referenced this pull request May 28, 2025
…t match workflow group matching regex (geonetwork#8699)

* add isProfileOrMoreForGroup method

* Hide enable workflow button if group name does not match regex

* Move workflow option display logic to function

* Hide workflow label when no options are shown

* Move configuration variables to CatController

* Hide workflow status label from editor board

* Prevent enabling workflow for groups with workflow disabled in API

* Refactor

* Hide warnPublishDraft confirmation when workflow is not enabled

* Add message

* Display status label if workflow is already enabled

* Prevent setting status from null to any status for records owned by groups with workflow disabled

* Add dutch messages

* Remove ES6 code
ianwallen pushed a commit to ianwallen/core-geonetwork that referenced this pull request May 29, 2025
…t match workflow group matching regex (geonetwork#8699)

* add isProfileOrMoreForGroup method

* Hide enable workflow button if group name does not match regex

* Move workflow option display logic to function

* Hide workflow label when no options are shown

* Move configuration variables to CatController

* Hide workflow status label from editor board

* Prevent enabling workflow for groups with workflow disabled in API

* Refactor

* Hide warnPublishDraft confirmation when workflow is not enabled

* Add message

* Display status label if workflow is already enabled

* Prevent setting status from null to any status for records owned by groups with workflow disabled

* Add dutch messages

* Remove ES6 code
ianwallen added a commit that referenced this pull request May 29, 2025
…t match workflow group matching regex (#8699) (#8831)

* add isProfileOrMoreForGroup method

* Hide enable workflow button if group name does not match regex

* Move workflow option display logic to function

* Hide workflow label when no options are shown

* Move configuration variables to CatController

* Hide workflow status label from editor board

* Prevent enabling workflow for groups with workflow disabled in API

* Refactor

* Hide warnPublishDraft confirmation when workflow is not enabled

* Add message

* Display status label if workflow is already enabled

* Prevent setting status from null to any status for records owned by groups with workflow disabled

* Add dutch messages

* Remove ES6 code

Co-authored-by: tylerjmchugh <[email protected]>
@@ -516,6 +517,31 @@ public Map<Integer, StatusChangeType> setStatus(@Parameter(description = API_PAR
.withDescriptionKey("exception.resourceNotEnabled.workflow.description");
}

// If the metadata workflow status is unset and the new status is DRAFT, workflow is being enabled
if (metadataStatus.getStatus(metadata.getId()) == null) {
Copy link
Member

Choose a reason for hiding this comment

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

This looks to affect task creation eg. when requesting creation of a DOI.

The following call:

curl 'http://localhost:8080/geonetwork/srv/api/records/1693/status' \
  -X 'PUT' \
  --data-raw '{"status":100,"owner":null,"dueDate":null,"changeMessage":""}'

is failing with

{
    "message": "Le workflow de groupe 'ABC' est désactivé",
    "code": "feature_disabled",
    "description": "Impossible de définir l'état des fiches dans le groupe 'ABC'"
}

but a task does not require to have the workflow mode enabled.

Maybe we should change

 // If the new status is not a task and the metadata workflow status is unset and the new status is DRAFT, workflow is being enabled
        if (metadataStatusValue.getStatusValue().getType() != StatusValueType.task
            && metadataStatus.getStatus(metadata.getId()) == null) {

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think your suggestion makes sense. But maybe we should do the following instead:

// If the metadata workflow status is unset and the new status is a workflow status, workflow is being enabled
if (metadataStatusValue.getStatusValue().getType() == StatusValueType.workflow
    && metadataStatus.getStatus(metadata.getId()) == null) {

This will ensure only workflow statuses are blocked

Copy link
Member

Choose a reason for hiding this comment

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

Added in #8857

fxprunayre added a commit to fxprunayre/core-geonetwork that referenced this pull request Jun 12, 2025
See geonetwork#8699 (comment)


Task status is blocked when workflow disabled eg. when requesting creation of a DOI.

The following call:

```
curl 'http://localhost:8080/geonetwork/srv/api/records/1693/status' \
  -X 'PUT' \
  --data-raw '{"status":100,"owner":null,"dueDate":null,"changeMessage":""}'
```

is failing with 

```
{
    "message": "Le workflow de groupe 'ABC' est désactivé",
    "code": "feature_disabled",
    "description": "Impossible de définir l'état des fiches dans le groupe 'ABC'"
}

```

but a task does not require to have the workflow mode enabled.
@josegar74
Copy link
Member

@tylerjmchugh I'm getting this kind of errors in the JS console when I access a metadata page:

angular.js?v=867f9c48f67de777c83da5cc3d514440438d19cb:15697 TypeError: Cannot read properties of undefined (reading 'reviewer')
    at scope.getStatusEffects (directive.js:351:39)
    at fn (eval at compile (angular.js?v=867f9c48f67de777c83da5cc3d514440438d19cb:16548:15), <anonymous>:4:235)
    at interceptedExpression (angular.js?v=867f9c48f67de777c83da5cc3d514440438d19cb:17682:55)
    at Scope.$digest (angular.js?v=867f9c48f67de777c83da5cc3d514440438d19cb:19262:34)
    at Scope.$apply (angular.js?v=867f9c48f67de777c83da5cc3d514440438d19cb:19630:24)
    at done (angular.js?v=867f9c48f67de777c83da5cc3d514440438d19cb:13473:47)
    at completeRequest (angular.js?v=867f9c48f67de777c83da5cc3d514440438d19cb:13730:7)
    at XMLHttpRequest.requestLoaded (angular.js?v=867f9c48f67de777c83da5cc3d514440438d19cb:13635:9)

It seems the scope variable scope.statusEffects is not defined yet when the page is loaded: https://github.com/geonetwork/core-geonetwork/blame/e3c7c98abbebe2d382762d2200ba60c1b177d1ec/web-ui/src/main/resources/catalog/views/default/directives/directive.js#L351

@fxprunayre the variable is initialised in a promise in https://github.com/geonetwork/core-geonetwork/blame/e3c7c98abbebe2d382762d2200ba60c1b177d1ec/web-ui/src/main/resources/catalog/views/default/directives/directive.js#L184, is there any reason for that? It doesn't use information from the data returned. Can not be initialised in the directive?

josegar74 added a commit to GeoCat/core-geonetwork that referenced this pull request Jun 20, 2025
@fxprunayre
Copy link
Member

is there any reason for that? It doesn't use information from the data returned. Can not be initialised in the directive?

I don't remember exactly, maybe in custom workflow like https://github.com/Informatievlaanderen/metadata-geonetwork/blob/develop/web-ui/src/main/resources/catalog/views/default/directives/directive.js#L167 but it is not needed also. Looks unnecessary. So we can move initialization @josegar74

fxprunayre pushed a commit to SPW-DIG/metawal-core-geonetwork that referenced this pull request Jun 23, 2025
josegar74 added a commit that referenced this pull request Jun 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants