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

UIIN-2546/UIIN-2548/UIIN-2549/UIIN-2556/UIIN-2557 Number generator implementation #2408

Open
wants to merge 28 commits into
base: master
Choose a base branch
from

Conversation

EthanFreestone
Copy link

@EthanFreestone EthanFreestone commented Mar 1, 2024

Purpose

Make use of the service interaction feature "number generator" to handle item barcode, accession number and call number fields in inventory.

This is an implementation to guide usage of the number generator library of components supplied by ui-service-interaction. This is not the only way to use them, and instead represents a guide on how they might be used, with the power of change living with the developer team, this is not a plugin over which you have 0 control, but rather an example implementation of this cross module functionality.

Approach

This comprises of 3 main "feature blocks"

Settings

New settings page - Number generator options.
This is a form containing two accordions, one for holding level interactions and one for item level interactions.

These set up simple checkboxes to turn on/off number generator for the fields in question, with 3 levels of configurability for the number generator:

  • off (field exists as before)
  • on without edit (field is disabled, the number generator button is rendered and is the only way to fill the field)
  • on (field is non disabled, can either be filled in manually or by using the generator)

For items there is an additional ability - use accession number for call number. This will allow the user to specify that the accession number sequences are also the options for call number field, and the two will share the same sequences and counts.

These settings currently make use of mod-configuration, but in reality so long as they can be stored and retrieved it does not matter how these settings are configured.

Side note- The sequences and generators can be configured in the settings page for "service interaction"

Holdings form

Added number generator button to call number field in holdings form

Item form

Added number generator button to call number, accession number and item barcode fields in items form

Initial setup of number generator in inventory, package changes and settings page set up
Added useConfigurationQuery hook in the same style as other hooks in use already, and made use of this in EditItem and CreateItem to pass to ItemForm.

Ideally this could just live in ItemForm, but that component is a class component and refactoring that to functional to use a hook is out of scope for this work.
Hooked up itemForm to the settings, small tweaks to settings to avoid softlocking user out of options, some translations
Tweaked settings layouts and translations (WIP, missing a translation still), and refactored ItemForm implementation to make simpler. Also moved majority of render logic to a separate utility method to keep the changes to the actual form component to a minimum
Added translation to the inofPopover in settings and small linting changes
Added safety feature to ensure that if the "use the same generated number for accession number and call number" did get set by accident when accession number and/or call number were "off", then it would ignore that setting
Manually provide styling for disabling of radio buttons since that isn't set up by default in stripes
Small tweaks to translations, added "learn more" button to link to documentation (when written)
Added labels to make modal implementation slightly nicer to view
Added some padding to the text at the top of the joint accession/call number modal, and removed it from the MessageBanner
Update to use servint 3
Added call number generator button to Holdings form, tweaks to settings
Changed Button from fullWidth to scale with text
Change order of ui-inventory NumGen settings

UIIN-2557
Copy link

github-actions bot commented Mar 1, 2024

Jest Unit Test Statistics

    1 files  ±0  231 suites  +1   12m 9s ⏱️ - 3m 50s
930 tests +1  928 ✔️ +1  2 💤 ±0  0 ±0 
936 runs  +1  934 ✔️ +1  2 💤 ±0  0 ±0 

Results for commit b0a16b6. ± Comparison against base commit ad11c96.

♻️ This comment has been updated with latest results.

@mariia-aloshyna
Copy link
Contributor

Hi @EthanFreestone could you add the description to the PR, please? Like the purpose, approach, and maybe some screenshots?

@EthanFreestone
Copy link
Author

EthanFreestone commented Mar 6, 2024

Hi @EthanFreestone could you add the description to the PR, please? Like the purpose, approach, and maybe some screenshots?

Hi @mariia-aloshyna I believe the purpose is covered by the linked JIRA tickets, a brief instruction on the approach is now part of the description.

Effectively though - we've created a library of components which can make use of the "number generator" functionality in mod-service-interaction. This is an example of how inventory might wish to implement that functionality, given the JIRA tickets raised.

The component library is highly configurable, and there are (as with all frontend work) many ways to skin this particular cat

@@ -26,6 +26,8 @@ const CreateHolding = ({
mutator,
}) => {
const callout = useCallout();
const { configs } = useConfigurationQuery('number_generator');
Copy link
Contributor

Choose a reason for hiding this comment

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

Move number_generator to constants, please, I see this is used in several places.

component={TextArea}
rows={1}
fullWidth
disabled={this.isFieldBlocked('callNumber') || callNumberGeneratorSettingHoldings === 'useGenerator'}
Copy link
Contributor

Choose a reason for hiding this comment

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

Move useGenerator string to constants, please

<Row>
{(
callNumberGeneratorSettingHoldings === 'useGenerator' ||
callNumberGeneratorSettingHoldings === 'useBoth'
Copy link
Contributor

Choose a reason for hiding this comment

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

And useBoth also

fullWidth
disabled={this.isFieldBlocked('callNumber')}
/>
<Row>
Copy link
Contributor

Choose a reason for hiding this comment

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

So currently Row is a child for Col and then there is another nested Col, is that how it was intended?

<Col xs={12}>
<NumberGeneratorModalButton
buttonLabel={<FormattedMessage id="ui-inventory.numberGenerator.generateCallNumber" />}
callback={(generated) => change('callNumber', generated)}
Copy link
Contributor

Choose a reason for hiding this comment

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

move this to a separate func

Comment on lines +17 to +28
const accessionNumberGeneratorActive = accessionNumberGeneratorSettingItems === 'useGenerator' ||
accessionNumberGeneratorSettingItems === 'useBoth';

const barcodeGeneratorActive = barcodeGeneratorSettingItems === 'useGenerator' ||
barcodeGeneratorSettingItems === 'useBoth';

const callNumberGeneratorActive = callNumberGeneratorSettingItems === 'useGenerator' ||
callNumberGeneratorSettingItems === 'useBoth';

const disableAccessionNumberField = accessionNumberGeneratorSettingItems === 'useGenerator';
const disableBarcodeField = barcodeGeneratorSettingItems === 'useGenerator';
const disableCallNumberField = callNumberGeneratorSettingItems === 'useGenerator';
Copy link
Contributor

Choose a reason for hiding this comment

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

All strings should be moved to constants and use the naming pattern, like if your variable is boolean, then call it isAccessionNumberGeneratorActive, isAccessionNumberFieldDisabled etc.

const query = `query=(module=INVENTORY and configName=${configName})`;

const queryKey = [namespace, query, 'useConfigurationQuery'];

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove extra empty line, please

import React from 'react';

import { screen } from '@folio/jest-config-stripes/testing-library/react';
import stripesFinalForm from '@folio/stripes/final-form';
Copy link
Contributor

Choose a reason for hiding this comment

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

There is renderWithFinalForm helper in ui-inventory, you can use it

Copy link
Contributor

Choose a reason for hiding this comment

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

Please, cover this component with unit tests

@@ -484,6 +485,14 @@
],
"visible": true
},
{
"permissionName": "ui-inventory.settings.numberGenerator.manage",
Copy link
Contributor

Choose a reason for hiding this comment

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

Add the new permission to translations as well

Copy link

sonarcloud bot commented Mar 28, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
57.3% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

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

Successfully merging this pull request may close these issues.

2 participants