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

UIORGS-336: Creating or changing a vendor code using number generator in Organizations App #536

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

Conversation

EthanFreestone
Copy link

feat: number generator

Adds the number generator functionality to Vendor code.
Also setting page to enable/disable this function, protected by a permission: ui-organizations.settings.numberGenerator.manage

Reflects the PR here:
folio-org/ui-users#2231
and cannot be merged until ui-service-interaction is accepted for a flower release

refs UXPROD-3891, UIORGS-336, UIORGS-337

Hooked up Number generator to Organizations
Added servint as an optionalOkapiInterface dependency, and protected code against being called when interface is not present
Small tweaks, brought number generator code in line with users PR

refs UIORGS-336, UIORGS-337
Update generator code to use the new codes defined in this PR:

folio-org/mod-service-interaction#72
Added new permission and blocked access to number generator page based on that permission
@github-actions
Copy link

github-actions bot commented Feb 16, 2023

Jest Unit Test Statistics

    1 files  ±0    84 suites  ±0   3m 1s ⏱️ -48s
268 tests ±0  267 ✔️  - 1  0 💤 ±0  1 +1 
272 runs  ±0  271 ✔️  - 1  0 💤 ±0  1 +1 

For more details on these failures, see this check.

Results for commit 5e4e4c2. ± Comparison against base commit fc155d5.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Feb 16, 2023

BigTest Unit Test Statistics

0 tests  ±0   0 ✔️ ±0   0s ⏱️ ±0s
0 suites ±0   0 💤 ±0 
0 files   ±0   0 ±0 

Results for commit bae809d. ± Comparison against base commit fc155d5.

♻️ This comment has been updated with latest results.

Update to use servint 3
Use new service-interaction
Swapped implementation to full Modal implementation as per the other apps
Changed Button from fullWidth to scale with text
Change back to do what is done on master
@EthanFreestone EthanFreestone marked this pull request as ready for review March 1, 2024 11:39
@EthanFreestone EthanFreestone marked this pull request as draft March 1, 2024 11:39
Mocked useSettings to rectify failing test
@EthanFreestone EthanFreestone marked this pull request as ready for review March 1, 2024 11:53
let vendorCodeSetting = 'useTextField';

if (stripes.hasInterface('servint')) {
vendorCodeSetting = settings?.find(sett => sett?.configName === CONFIG_NAME)?.parsedSettings?.vendorGeneratorSetting ?? 'useTextField';
Copy link
Contributor

Choose a reason for hiding this comment

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

this and related code should be moved to hook

/>
</Col>
{(
vendorCodeSetting === 'useGenerator' ||
Copy link
Contributor

Choose a reason for hiding this comment

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

plz don't use magic words, additionally IMO you can return boolean values from hook (see previous comment)

generateButtonLabel={<FormattedMessage id="ui-organizations.numberGenerator.generateVendorCode" />}
generator="organizations_vendorCode"
modalProps={{
label: <FormattedMessage id="ui-organizations.numberGenerator.vendorCodeGenerator" />
Copy link
Contributor

Choose a reason for hiding this comment

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

fix plinter please

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

Choose a reason for hiding this comment

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

it would be nice to not create functions and objects in return section

}));

jest.mock('@folio/service-interaction', () => ({
NumberGeneratorModalButton: () => <div>NumberGeneratorModalButton</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

fix linter please

const NumberGeneratorOptions = (props) => {
const ConnectedConfigManager = stripesConnect(ConfigManager);

const defaultValues = {
Copy link
Contributor

Choose a reason for hiding this comment

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

move it outside of component as it's just constant

@@ -8,4 +8,6 @@ export const PRIVILEGED_CONTACTS_API = 'organizations-storage/privileged-contact
export const SETTINGS_API = 'organizations-storage/settings';
export const TYPES_API = 'organizations-storage/organization-types';

export const CONFIG_API = 'configurations/entries';
Copy link
Contributor

Choose a reason for hiding this comment

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

configurations is deprecated and shouldn't be used for new functionality

Copy link
Contributor

@NikitaSedyx NikitaSedyx left a comment

Choose a reason for hiding this comment

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

  1. low level of coverage
  2. mod-configration shouldn't be used for new functionality
  3. code should be refactored

Copy link

sonarcloud bot commented Mar 28, 2024

Quality Gate Failed Quality Gate failed

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

See analysis details on SonarCloud

Moved connected component outside of component render, removed default values
Added messageBanner to settings page explaining the number generator options
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