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

UIBULKED-605 Enabling Confirm changes button based on forms state #679

Merged
merged 8 commits into from
Jan 28, 2025
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
* [UIBULKED-599](https://folio-org.atlassian.net/browse/UIBULKED-599) Change Administrative note type is not supported for MARC instances.
* [UIBULKED-589](https://folio-org.atlassian.net/browse/UIBULKED-589) Make options in the "Actions" dropdown in "Bulk edits" in alphabetical order.
* [UIBULKED-588](https://folio-org.atlassian.net/browse/IBULKED-588) Displaying errors and warnings.
* [UIBULKED-605](https://folio-org.atlassian.net/browse/UIBULKED-605) Enabling Confirm changes button based on forms state.

## [4.2.2](https://github.com/folio-org/ui-bulk-edit/tree/v4.2.2) (2024-11-15)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,10 @@ export const isContentUpdatesFormValid = (contentUpdates) => {
});
};

export const isMarcContentUpdatesFormValid = (errors) => {
return Object.keys(errors).length === 0;
};

export const getFilteredFields = (initialFields) => {
return initialFields.map(f => {
const uniqOptions = new Set(initialFields.map(i => i.option));
Expand Down
59 changes: 32 additions & 27 deletions src/components/BulkEditPane/BulkEditMarcLayer/BulkEditMarcLayer.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import React, { useState } from 'react';
import PropTypes from 'prop-types';
import { useIntl } from 'react-intl';
import PropTypes from 'prop-types';
import uniqueId from 'lodash/uniqueId';
import { omit, isEqual } from 'lodash';

import { BulkEditLayer } from '../BulkEditListResult/BulkEditInAppLayer/BulkEditLayer';
import { BulkEditMarc } from '../BulkEditListResult/BulkEditMarc/BulkEditMarc';
Expand All @@ -13,14 +14,20 @@ import { useContentUpdate } from '../../../hooks/api';
import {
getContentUpdatesBody,
getMappedContentUpdates,
isContentUpdatesFormValid
isContentUpdatesFormValid,
isMarcContentUpdatesFormValid
} from '../BulkEditListResult/BulkEditInApp/ContentUpdatesForm/helpers';
import { getMarcFormErrors } from '../BulkEditListResult/BulkEditMarc/validation';
import { getAdministrativeDataOptions } from '../../../constants';
import { sortAlphabetically } from '../../../utils/sortAlphabetically';
import { BulkEditPreviewModalFooter } from '../BulkEditListResult/BulkEditInAppPreviewModal/BulkEditPreviewModalFooter';
import { useCommitChanges } from '../../../hooks/useCommitChanges';

import { getAdministrativeDataOptions } from '../../../constants';
import { getMarcFormErrors } from '../BulkEditListResult/BulkEditMarc/validation';
import {
ADMINISTRATIVE_DEFAULT_BODY,
ADMINISTRATIVE_FORM_INITIAL_STATE,
MARC_DEFAULT_BODY,
MARC_FORM_INITIAL_STATE
} from '../../../constants/forms';

export const BulkEditMarcLayer = ({
bulkOperationId,
Expand All @@ -40,11 +47,20 @@ export const BulkEditMarcLayer = ({
const sortedOptions = sortAlphabetically(options);

const marcFormErrors = getMarcFormErrors(marcFields);
const marcContentUpdates = marcFields.map(getTransformedField);
const marcContentUpdatesWithoutId = marcContentUpdates.map(item => omit(item, ['id']));
const contentUpdates = getMappedContentUpdates(fields, options);

const isMarcFieldsValid = Object.keys(marcFormErrors).length === 0;
const isMarcFormValid = isMarcContentUpdatesFormValid(marcFormErrors);
const isAdministrativeFormValid = isContentUpdatesFormValid(contentUpdates);
const isAnyFormValid = isMarcFieldsValid || isAdministrativeFormValid;

const isAdministrativeFormPristine = !isEqual(ADMINISTRATIVE_FORM_INITIAL_STATE, contentUpdates);
const isMarcFormPristine = !isEqual(MARC_FORM_INITIAL_STATE, marcContentUpdatesWithoutId);

// state is valid if at least one form is filled and valid and another one is not pristine or both forms are filled and valid

Choose a reason for hiding this comment

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

I think you meant to say in the comment that another one is pristine

Copy link
Member

Choose a reason for hiding this comment

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

I'm with @BogdanDenis here, but I feel like both the comment AND the conditional contradict the PR description which states "2 valid forms, or 1 valid and one untouched". Here, you instead check 2 valid forms or 1 valid and one NOT pristine. Or do you have the booleans on lines 57 and 58 configured backwards, i.e. isPristine should be true when those values are equal, instead of when they differ?

Super petty: since your comment describes two different scenarios, just put them on two different lines. As-is, it's hard to parse how the ands and ors separate things.

const areFormsStateValid = (isAdministrativeFormValid && !isMarcFormPristine)
|| (isMarcFormValid && !isAdministrativeFormPristine)
|| (isAdministrativeFormValid && isMarcFormValid);

const {
isPreviewModalOpened,
Expand All @@ -69,32 +85,21 @@ export const BulkEditMarcLayer = ({
const areMarcAndCsvReady = hasBothFiles && isPreviewSettled;

const handleConfirm = () => {
const bulkOperationMarcRules = marcFields
.map(field => ({
bulkOperationId,
...getTransformedField(field),
}));

const marcDefaultBody = {
bulkOperationMarcRules: [],
totalRecords: 0,
};

const administrativeDefaultBody = {
bulkOperationRules: [],
totalRecords: 0,
};

const marcUpdateBody = isMarcFieldsValid ? {
const bulkOperationMarcRules = marcContentUpdates.map((item) => ({
bulkOperationId,
...item
}));

const marcUpdateBody = isMarcFormValid ? {
bulkOperationMarcRules,
totalRecords,
} : marcDefaultBody;
} : MARC_DEFAULT_BODY;

const administrativeBody = isAdministrativeFormValid ? getContentUpdatesBody({
bulkOperationId,
contentUpdates,
totalRecords,
}) : administrativeDefaultBody;
}) : ADMINISTRATIVE_DEFAULT_BODY;

const updateSequence = () => contentUpdate(administrativeBody)
.then(() => marcContentUpdate(marcUpdateBody));
Expand All @@ -106,7 +111,7 @@ export const BulkEditMarcLayer = ({
<>
<BulkEditLayer
isLayerOpen={isMarcLayerOpen}
isConfirmDisabled={!isAnyFormValid}
isConfirmDisabled={!areFormsStateValid}
onLayerClose={onMarcLayerClose}
onConfirm={handleConfirm}
{...paneProps}
Expand Down
41 changes: 41 additions & 0 deletions src/constants/forms.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
export const MARC_DEFAULT_BODY = {
bulkOperationMarcRules: [],
totalRecords: 0,
};

export const ADMINISTRATIVE_DEFAULT_BODY = {
bulkOperationRules: [],
totalRecords: 0,
};

export const MARC_FORM_INITIAL_STATE = [
{
tag: '',
ind1: '\\',
ind2: '\\',
subfield: '',
actions: [{
name: '',
data: []
}],
parameters: [],
subfields: [],
}
];

export const ADMINISTRATIVE_FORM_INITIAL_STATE = [
{
option: '',
tenants: [],
actions: [
{
initial: undefined,
updated: undefined,
type: undefined,
tenants: [],
updated_tenants: [],
parameters: []
}
]
}
];
Loading