[UIU-3259] - Fix permission changes for mod-circulation

[azizbekxm]

Purpose

https://folio-org.atlassian.net/browse/UIU-3259

Approach

• change permssion
  it will be merged after prs
  [CIRC-2139] - Review and cleanup Module Descriptors for mod-circulation mod-circulation#1508
  [MODFEE-385] - Review and cleanup Module Descriptors for mod-feesfines mod-feesfines#299

[zburke]

• Please create a UIU ticket for this work and include that jira slug, e.g. UIU-999, in the title of this PR. We need a ticket in UIU, not CIRC, in order to correctly set the release-version in Jira.
• Please include a note in the CHANGELOG.
• If this work requires merging another PR, that implies this will be a breaking change to this module**. In this case, you must update package.json as well to bump the version and update the new minimum versions of the okapi interfaces that are changing. If you're making breaking changes to a module where you don't normally do work, it would be a courtesy to tag the lead dev for awareness. In this case, it's @Terala-Priyanka . See the project responsibility matrix for a list of repos/devs/etc.

** Corollary: there be a version bump to the interface that provides these new permissions. I don't see any version changes in the corresponding PR.

[Terala-Priyanka]

• Please create a UIU ticket for this work and include that jira slug, e.g. UIU-999, in the title of this PR. We need a ticket in UIU, not CIRC, in order to correctly set the release-version in Jira.
• Please include a note in the CHANGELOG.
• If this work requires merging another PR, that implies this will be a breaking change to this module**. In this case, you must update package.json as well to bump the version and update the new minimum versions of the okapi interfaces that are changing. If you're making breaking changes to a module where you don't normally do work, it would be a courtesy to tag the lead dev for awareness. In this case, it's @Terala-Priyanka . See the project responsibility matrix for a list of repos/devs/etc.

** Corollary: there be a version bump to the interface that provides these new permissions. I don't see any version changes in the corresponding PR.

Thank you for the note Zak.
@azizbekxm Please follow Zak's instructions.

[azizbekxm]

• Please create a UIU ticket for this work and include that jira slug, e.g. UIU-999, in the title of this PR. We need a ticket in UIU, not CIRC, in order to correctly set the release-version in Jira.
• Please include a note in the CHANGELOG.
• If this work requires merging another PR, that implies this will be a breaking change to this module**. In this case, you must update package.json as well to bump the version and update the new minimum versions of the okapi interfaces that are changing. If you're making breaking changes to a module where you don't normally do work, it would be a courtesy to tag the lead dev for awareness. In this case, it's @Terala-Priyanka . See the project responsibility matrix for a list of repos/devs/etc.

** Corollary: there be a version bump to the interface that provides these new permissions. I don't see any version changes in the corresponding PR.

Thanks, fixed updated interface for mod-feesfines

[Terala-Priyanka]

@zburke 19.0 exists already. Should 19.1 explicitly added here?

[zburke]

It depends :) "feesfines": "16.1 17.0 18.0 19.0 19.1" means "this module can run just fine when any of these interfaces are present". In this context, 19.1 is redundant to 19.0 because the semver equivalent of 19.0 is ^19.

But that's not the most important question, which is, "Given this PR removes many permissions from existing permission-sets, what will the impact be?" e.g. the user-assignable pset ui-users.feesfines.actions.all has accounts.transfer.post replaced with feesfines.accounts.transfer.item.post. What would happen in an environment where only 19.0 (or only 16.1!) is provided? Will the user be able to do the same things before and after this change? I think the answer is "No" because this PR removes accounts.transfer.post (even though exists on the backend in v19.0) and replaces it with feesfines.accounts.transfer.item.post (which does NOT exist on the backend in v19.0). IOW, removing permissions from existing psets is a breaking change.

There are two options:

1. Accept the breaking change: Replace old permissions with new ones as the PR currently does, AND bump the minimum interfaces (e.g. "feesfines": "19.1"), AND bump the major version of this repository
2. Do this work in a backwards-compatible way: Add new permissions along-side the old permissions, i.e. instead of replacing accounts.transfer.post with feesfines.accounts.transfer.item.post, include them both in the same pset. I believe it's possible to do this, and mod-permissions will simply ignore the permissions that don't exist (i.e. in a 19.0 env it will ignore the new perm that doesn't exist (because it has not been created yet), and in a 19.1 env it will ignore the old perm that doesn't exist (because it has already been removed)). Please confirm this behavior with somebody who works on mod-permissions, maybe @julianladisch? He is tech-lead on that module.

Finally, please confirm that these new permissions are all correct. The one I happened to pick as an example, feesfines.accounts.transfer.item.post, does not appear in either of the two linked PRs.

[azizbekxm]

1. removed 19.0 and added 19.1
2. increased to version 11.1.0
3. Updated permissions one more time

[zburke]

"removed 19.0 and added 19.1" is incorrect. If you are removing 19.0, you also need to remove 16.0 and all the others, and you must bump the MAJOR version of this package because removing support for an interface is a breaking change.

[Terala-Priyanka]

Hey @azizbekxm
This is not the correct way of updating the changelog in your case.

[azizbekxm]

thanks for suggestion, fixed this

[sonarcloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[zburke]

@azizbekxm, @Terala-Priyanka, if this change is intended to be a part of Ramsons, which is already a breaking release, then we don't need to bump the major version again, but we do need to correctly set the minimum version of the okapi-interfaces that provide these permissions.

Yeah yeah, technically that means we're putting a breaking change in a patch release, which seems awful, but pre-bugfest, I think of all the "releases" as "-alpha" releases, IOW, v11.0.0 is really v11.0.0-alpha-1 and after this merge we'd release v11.0.0-alpha-2 and finally when bugfest was all finished and we're ready to certify Ramsons, then we'd finally tag v11.0.0. If that's what's happening here -- we are trying to cram something in that was supposed to be there before the v11.0.0 release was ever tagged -- we should just do it and I'll get off my high "but Semver!" horse. But you still have to set the minimum okapi-interface versions correctly :)