Skip to content

UICIRC-942: Reminder print files#1110

Merged
mkuklis merged 19 commits into
masterfrom
UICIRC-942
Jan 8, 2024
Merged

UICIRC-942: Reminder print files#1110
mkuklis merged 19 commits into
masterfrom
UICIRC-942

Conversation

@mkuklis
Copy link
Copy Markdown
Contributor

@mkuklis mkuklis commented Dec 13, 2023

https://issues.folio.org/browse/UICIRC-942

Add Patron notice print jobs to circulation settings.

patron-notice-print-jobs

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Dec 13, 2023

Jest Unit Test Statistics

       1 files  ±0     171 suites  +1   1m 57s ⏱️ +13s
1 849 tests +5  1 849 ✔️ +5  0 💤 ±0  0 ±0 
1 887 runs  +5  1 887 ✔️ +5  0 💤 ±0  0 ±0 

Results for commit e40ece5. ± Comparison against base commit 1532394.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Dec 13, 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 e40ece5. ± Comparison against base commit 1532394.

♻️ This comment has been updated with latest results.

@mkuklis mkuklis marked this pull request as ready for review January 3, 2024 19:05
@mkuklis mkuklis requested a review from MikeTaylor January 4, 2024 03:56
Copy link
Copy Markdown
Contributor

@MikeTaylor MikeTaylor left a comment

Choose a reason for hiding this comment

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

This looks reasonable. I just worry about dependency proliferation.

Comment thread package.json
Comment thread src/index.js
route: 'patron-notice-print-jobs',
label: this.props.intl.formatMessage({ id: 'ui-circulation.settings.index.patronNoticePrintJobs' }),
component: PatronNoticePrintJobs,
perm: 'mod-batch-print.entries.collection.get',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Once we finally merge folio-org/stripes-smart-components#1401 you'll be able to also add here:

iface: 'batch-print',

to support optionality.

Comment thread src/settings/PatronNoticePrintJobs/PatronNoticePrintJobs.js Outdated
@@ -0,0 +1 @@
jest.mock('currency-codes/data', () => ({ filter: () => [] }));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this related to the main purpose of the PR?

(Not a killer if it isn't, I just want to understand.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@MikeTaylor yes. I needed to use MCL in the tests (not a mocked MCL) but then started getting errors related to currency codes. By including a mock I was able to bypass it. A similar pattern is used in most other UI modules.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Makes sense!

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Jan 8, 2024

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

3 New issues
0 Security Hotspots
81.4% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@mkuklis mkuklis merged commit 6537a79 into master Jan 8, 2024
mkuklis added a commit that referenced this pull request Jan 8, 2024
* UICIRC-942: Add new page for access of reminder print files

* UICIRC-942: Connect Print UI to the backend

* fix print jobs

* Adjust print jobs

* Cleanup

* Add tests

* Use new API for mutli removal

* Fix tests

* Cleanup

* Cleanup

* Add more tests

* Update permission

* batch-print to optionalOkapiInterfaces

* Use callout for errors

* Fix tests
@MikeTaylor
Copy link
Copy Markdown
Contributor

Thanks, @mkuklis, this all looks better now. I'm always taken by surprise at how easy it is to use callouts!

I do think though that you need to protect your top-level settings page so that the new settings are only included when the appropriate interface is provided.

mkuklis added a commit that referenced this pull request Jan 24, 2024
mkuklis added a commit that referenced this pull request Jan 24, 2024
mkuklis added a commit that referenced this pull request Jan 24, 2024
@Dmitriy-Litvinenko Dmitriy-Litvinenko deleted the UICIRC-942 branch March 12, 2025 14:32
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