Skip to content

Refactor filters#293

Merged
aatuny merged 10 commits intotestfrom
refactor-filters
Nov 24, 2025
Merged

Refactor filters#293
aatuny merged 10 commits intotestfrom
refactor-filters

Conversation

@natlibfi-jonollil
Copy link
Copy Markdown
Member

No description provided.

@natlibfi-jonollil natlibfi-jonollil requested a review from a team as a code owner November 19, 2025 13:16
Copy link
Copy Markdown
Contributor

@aatuny aatuny left a comment

Choose a reason for hiding this comment

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

Good work and code quality is on par with current repo code quality.

Asking for few improvements:

  • check msg/result of year filter tests as I observed some inconsistency
  • use example.com TLD in tests
  • do not allow ISBN from URN (at least yet, until we have confirmation from metadata expert on this subject)
  • add JSDoc style comments at least for the ISBN-filtering function as reverse parameter might be difficult to interpret

Comment thread src/transform/filter/filterByIsbnIdentifier.js
Comment thread src/transform/filter/filterByIsbnIdentifier.js Outdated
Comment thread test-fixtures/transform/filter/filterByFileType/01/input.xml Outdated
Comment thread test-fixtures/transform/filter/filterByIsbnIdentifier/05/metadata.json Outdated
Comment thread test-fixtures/transform/filter/filterByIssuedYear/03/input.xml
Comment thread test-fixtures/transform/filter/filterByIssuedYear/06/output.json
Comment thread src/constants.js Outdated
@aatuny aatuny merged commit 2282368 into test Nov 24, 2025
5 checks passed
@aatuny aatuny deleted the refactor-filters branch November 24, 2025 11:12
aatuny added a commit that referenced this pull request Dec 17, 2025
* Refactor filters (#293)
* Update actions
* Update dependencies
* 3.1.0-alpha.1

---------

Co-authored-by: Joni Ollila <53558796+natlibfi-jonollil@users.noreply.github.com>
Co-authored-by: aatuny <27770287+aatuny@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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