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

UISAUTHCOM-1: Move reusable components from UIROLES #2

Merged
merged 16 commits into from
Jul 8, 2024

Conversation

alisher-epam
Copy link
Contributor

@alisher-epam alisher-epam commented Jun 28, 2024

Purpose

UISAUTHCOM-1: Move reusable components from UIROLES

Approach

Pre-Merge Checklist

Before merging this PR, please go through the following list and take appropriate actions.

  • I've added appropriate record to the CHANGELOG.md
  • Does this PR meet or exceed the expected quality standards?
    • Code coverage on new code is 80% or greater
    • Duplications on new code is 3% or less
    • There are no major code smells or security issues
  • Does this introduce breaking changes?
    • If any API-related changes - okapi interfaces and permissions are reviewed/changed correspondingly
    • There are no breaking changes in this PR.

If there are breaking changes, please STOP and consider the following:

  • What other modules will these changes impact?
  • Do JIRAs exist to update the impacted modules?
    • If not, please create them
    • Do they contain the appropriate level of detail? Which endpoints/schemas changed, etc.
    • Do they have all they appropriate links to blocked/related issues?
  • Are the JIRAs under active development?
    • If not, contact the project's PO and make sure they're aware of the urgency.
  • Do PRs exist for these changes?
    • If so, have they been approved?

Ideally all of the PRs involved in breaking changes would be merged in the same day to avoid breaking the folio-testing environment. Communication is paramount if that is to be achieved, especially as the number of intermodule and inter-team dependencies increase.

While it's helpful for reviewers to help identify potential problems, ensuring that it's safe to merge is ultimately the responsibility of the PR assignee.

@alisher-epam alisher-epam self-assigned this Jun 28, 2024
Copy link

github-actions bot commented Jun 28, 2024

Jest Unit Test Statistics

    1 files  ±  0    36 suites  +29   1m 27s ⏱️ +48s
  97 tests +80    97 ✔️ +80  0 💤 ±0  0 ±0 
101 runs  +84  101 ✔️ +84  0 💤 ±0  0 ±0 

Results for commit 62e839d. ± Comparison against base commit 12d5592.

This pull request removes 4 and adds 84 tests. Note that renamed tests count towards both.
Procedural capabilities type should render null ‑ Procedural capabilities type should render null
Settings capabilities type renders checkboxes ‑ Settings capabilities type renders checkboxes
Settings capabilities type renders fields in the grid ‑ Settings capabilities type renders fields in the grid
Settings capabilities type renders null if action name is not compatible with view, edit, manage actions ‑ Settings capabilities type renders null if action name is not compatible with view, edit, manage actions
CapabilitiesSection should render `CapabilitiesDataType` ‑ CapabilitiesSection should render `CapabilitiesDataType`
CapabilitiesSection should render `CapabilitiesDataType`, `CapabilitiesProcedural`, and `CapabilitiesSettings` ‑ CapabilitiesSection should render `CapabilitiesDataType`, `CapabilitiesProcedural`, and `CapabilitiesSettings`
CapabilitiesSection should render nothing if there is no capabilities ‑ CapabilitiesSection should render nothing if there is no capabilities
CapabilitiesSettings renders checkboxes ‑ CapabilitiesSettings renders checkboxes
CapabilitiesSettings renders fields in the grid ‑ CapabilitiesSettings renders fields in the grid
CapabilitiesSettings renders null if action name is not compatible with view, edit, manage actions ‑ CapabilitiesSettings renders null if action name is not compatible with view, edit, manage actions
Procedural capabilities type renders null if action name is not execute ‑ Procedural capabilities type renders null if action name is not execute
RoleCreate actions on click footer buttons ‑ RoleCreate actions on click footer buttons
RoleCreate changes name, description input values ‑ RoleCreate changes name, description input values
RoleCreate renders TextField and Button components ‑ RoleCreate renders TextField and Button components
…

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Jun 28, 2024

BigTest Unit Test Statistics

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

Results for commit 62e839d. ± Comparison against base commit 12d5592.

♻️ This comment has been updated with latest results.

@alisher-epam alisher-epam force-pushed the UISAUTHCOM-1 branch 2 times, most recently from 35a5f1d to 0027de7 Compare July 3, 2024 15:24
@alisher-epam
Copy link
Contributor Author

@folio-org/fe-tl-reviewers review it please

@alisher-epam
Copy link
Contributor Author

@usavkov-epam @BogdanDenis please check the PR when you are available

@usavkov-epam
Copy link
Contributor

I'm not sure you need to create empty translation files, except en.json. I suppose the localization job will handle it on its own.

@alisher-epam
Copy link
Contributor Author

I'm not sure you need to create empty translation files, except en.json. I suppose the localization job will handle it on its own.

I suppose it came from a branch update from the master branch

@usavkov-epam
Copy link
Contributor

Before merging, please make sure one of the approvals belongs to developers from the Eureka team.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a Capabilities-specific component, or can it be reused in other entities? If it's common, it makes sense to move it outside of Capabilities/.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right now it is used with Capabilities

lib/Role/RoleCreate/RoleCreate.test.js Outdated Show resolved Hide resolved
Copy link
Contributor

@usavkov-epam usavkov-epam left a comment

Choose a reason for hiding this comment

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

Besides comments below, I don't see a reason to add compiled translations


const columnMapping = useMemo(() => {
return getColumnMapping(formatMessage);
}, []);
Copy link
Contributor

Choose a reason for hiding this comment

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

dependency missed

Comment on lines 42 to 55
useApplicationCapabilities.mockReturnValue({ capabilities: { data:[{ id:'6e59c367-888a-4561-a3f3-3ca677de437f',
applicationId:'app-platform-complete-0.0.5',
resource:'Erm Agreements Collection',
actions:{ view:'6e59c367-888a-4561-a3f3-3ca677de437f' } },
],
procedural:[],
settings:[{ id:'DDD-888a-4561-a3f3-3ca677de437f',
applicationId:'app-platform-complete-0.0.5',
resource:'Erm Agreements Collection',
actions:{ view:'DDD-888a-4561-a3f3-3ca677de437f' } }] },
roleCapabilitiesListIds: ['5c5198f9-de27-4349-9537-dc0b2b41c8c3'],
selectedCapabilitiesMap: { },
setSelectedCapabilitiesMap:mockSetSelectedCapabilitiesMap,
isLoading: false });
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
useApplicationCapabilities.mockReturnValue({ capabilities: { data:[{ id:'6e59c367-888a-4561-a3f3-3ca677de437f',
applicationId:'app-platform-complete-0.0.5',
resource:'Erm Agreements Collection',
actions:{ view:'6e59c367-888a-4561-a3f3-3ca677de437f' } },
],
procedural:[],
settings:[{ id:'DDD-888a-4561-a3f3-3ca677de437f',
applicationId:'app-platform-complete-0.0.5',
resource:'Erm Agreements Collection',
actions:{ view:'DDD-888a-4561-a3f3-3ca677de437f' } }] },
roleCapabilitiesListIds: ['5c5198f9-de27-4349-9537-dc0b2b41c8c3'],
selectedCapabilitiesMap: { },
setSelectedCapabilitiesMap:mockSetSelectedCapabilitiesMap,
isLoading: false });
useApplicationCapabilities.mockReturnValue({
capabilities: {
data: [{
id: '6e59c367-888a-4561-a3f3-3ca677de437f',
applicationId: 'app-platform-complete-0.0.5',
resource: 'Erm Agreements Collection',
actions: { view: '6e59c367-888a-4561-a3f3-3ca677de437f' }
}],
procedural: [],
settings: [{
id: 'DDD-888a-4561-a3f3-3ca677de437f',
applicationId: 'app-platform-complete-0.0.5',
resource: 'Erm Agreements Collection',
actions: { view: 'DDD-888a-4561-a3f3-3ca677de437f' }
}],
},
roleCapabilitiesListIds: ['5c5198f9-de27-4349-9537-dc0b2b41c8c3'],
selectedCapabilitiesMap: {},
setSelectedCapabilitiesMap: mockSetSelectedCapabilitiesMap,
isLoading: false,
});

Comment on lines 47 to 52
const paneFooterRenderStart = <Button
marginBottom0
buttonStyle="default mega"
onClick={onClose}
><FormattedMessage id="stripes-authorization-components.crud.cancel" />
</Button>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const paneFooterRenderStart = <Button
marginBottom0
buttonStyle="default mega"
onClick={onClose}
><FormattedMessage id="stripes-authorization-components.crud.cancel" />
</Button>;
const paneFooterRenderStart = (
<Button
marginBottom0
buttonStyle="default mega"
onClick={onClose}
>
<FormattedMessage id="stripes-authorization-components.crud.cancel" />
</Button>;
);

Comment on lines 54 to 60
const paneFooterRenderEnd = <Button
marginBottom0
buttonStyle="primary mega"
disabled={!roleName || isLoading}
type="submit"
onClick={onSubmit}
><FormattedMessage id="stripes-components.saveAndClose" /></Button>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const paneFooterRenderEnd = <Button
marginBottom0
buttonStyle="primary mega"
disabled={!roleName || isLoading}
type="submit"
onClick={onSubmit}
><FormattedMessage id="stripes-components.saveAndClose" /></Button>;
const paneFooterRenderEnd = (
<Button
marginBottom0
buttonStyle="primary mega"
disabled={!roleName || isLoading}
type="submit"
onClick={onSubmit}
>
<FormattedMessage id="stripes-components.saveAndClose" />
</Button>
);

const intl = useIntl();

return <form onSubmit={onSubmit} data-testid="create-role-form">
<Layer isOpen inRootSet contentLabel={intl.formatMessage({ id: title })}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<Layer isOpen inRootSet contentLabel={intl.formatMessage({ id: title })}>
<Layer
isOpen
inRootSet
contentLabel={intl.formatMessage({ id: title })}
>

Comment on lines 10 to 13
const { data, isLoading, isSuccess } = useQuery(
namespace,
() => ky.get(`groups?limit=${stripes.config.maxUnpagedResourceCount}&query=cql.allRecords=1 sortby desc`).json(),
);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const { data, isLoading, isSuccess } = useQuery(
namespace,
() => ky.get(`groups?limit=${stripes.config.maxUnpagedResourceCount}&query=cql.allRecords=1 sortby desc`).json(),
);
const { data, isLoading, isSuccess } = useQuery({
queryKey: [namespace],
queryFn: ({ signal }) => ky.get(`groups?limit=${stripes.config.maxUnpagedResourceCount}&query=cql.allRecords=1 sortby desc`, { signal }).json(),
});

@@ -0,0 +1,20 @@
import { useQuery } from 'react-query';

import { useNamespace, useOkapiKy, useStripes } from '@folio/stripes/core';
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
import { useNamespace, useOkapiKy, useStripes } from '@folio/stripes/core';
import {
useNamespace,
useOkapiKy,
useStripes,
} from '@folio/stripes/core';

Comment on lines 1 to 5
import { keyBy } from 'lodash';
import {
useChunkedCQLFetch,
useNamespace,
} from '@folio/stripes/core';
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
import { keyBy } from 'lodash';
import {
useChunkedCQLFetch,
useNamespace,
} from '@folio/stripes/core';
import keyBy from 'lodash/keyBy';
import {
useChunkedCQLFetch,
useNamespace,
} from '@folio/stripes/core';

Comment on lines 1 to 2
import React from 'react';
import { render } from '@folio/jest-config-stripes/testing-library/react';
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
import React from 'react';
import { render } from '@folio/jest-config-stripes/testing-library/react';
import { render } from '@folio/jest-config-stripes/testing-library/react';

@@ -0,0 +1,12 @@
import React from 'react';
import { Router } from 'react-router-dom';
import { createMemoryHistory } from 'history';
Copy link
Contributor

Choose a reason for hiding this comment

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

linter error: not listed dependency

Copy link

sonarcloud bot commented Jul 8, 2024

@alisher-epam
Copy link
Contributor Author

@usavkov-epam thank you for the feedback. I have fixed the codebase based on your comments.

@alisher-epam alisher-epam merged commit 4c188fc into master Jul 8, 2024
6 checks passed
@alisher-epam alisher-epam deleted the UISAUTHCOM-1 branch July 8, 2024 11:18
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.

5 participants