Skip to content

Conversation

@mhamann-ubl
Copy link
Contributor

Description

These changes add the possibility to pass a customItemFormatter to the AuditLogModal and add to this Formatter the collectionName.

Why is this change necessary?

While adding additionalCallNumbers to the inventorys item history as requested per this user story: https://folio-org.atlassian.net/browse/UIIN-3540 we noticed, that objects are currently being displayed as unordered, unnamed lists. This seems to be deficient as the user has to guess which value belongs to which property.

By adding a custom formatter, we are able to display these objects like this:

Screenshot from 2025-12-11 15-54-34

Related changes

Mod-Audit

folio-org/mod-audit#229

UI-Inventory

folio-org/ui-inventory#2950

@mhamann-ubl mhamann-ubl requested a review from a team as a code owner December 11, 2025 15:05
@github-actions
Copy link

github-actions bot commented Dec 11, 2025

Bigtest Unit Test Results

    1 files  ±0      1 suites  ±0   24s ⏱️ ±0s
1 623 tests +2  1 615 ✅ +2  8 💤 ±0  0 ❌ ±0 
1 624 runs  +2  1 616 ✅ +2  8 💤 ±0  0 ❌ ±0 

Results for commit ec3e2be. ± Comparison against base commit 58e8eb0.

♻️ This comment has been updated with latest results.

@zburke zburke changed the title Stcom 1475 STCOM-1475 AuditLogModal: accept customItemFormatter prop Dec 11, 2025
Copy link
Member

@zburke zburke left a comment

Choose a reason for hiding this comment

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

LGTM. In an ideal world, there would be a test showing custom vs default formatting with vs without this prop to prove it gets drilled through correctly. The changes here are not elaborate though; this may not be that world.

When you commit, please make sure the Jira slug in the commit-title correctly matches the story ("STCOM-1475", not "Stcom 1475") and that the commit title contains a useful summary of the work.

@mhamann-ubl
Copy link
Contributor Author

LGTM. In an ideal world, there would be a test showing custom vs default formatting with vs without this prop to prove it gets drilled through correctly. The changes here are not elaborate though; this may not be that world.

When you commit, please make sure the Jira slug in the commit-title correctly matches the story ("STCOM-1475", not "Stcom 1475") and that the commit title contains a useful summary of the work.

Thanks and sorry, was opened in a bit of a rush. It will be my last day working for folio, so I won't be able to add those tests.

@elsenhans
Copy link
Contributor

LGTM. In an ideal world, there would be a test showing custom vs default formatting with vs without this prop to prove it gets drilled through correctly. The changes here are not elaborate though; this may not be that world.

Still using karma tests? Shouldn't they be replaced by jest?

@elsenhans
Copy link
Contributor

@zburke I added tests for the itemFormatter

@elsenhans elsenhans requested review from a team and zburke December 15, 2025 11:07
@zburke
Copy link
Member

zburke commented Dec 15, 2025

@elsenhans, stripes-components and stripes-smart-compoents, which are UI-heavy as opposed to business-logic-heavy, use browser-based tests (yes, still karma 😢, hopefully Cypress some day) instead of jest. We have deliberately chosen to keep things this way.

Copy link
Member

@zburke zburke left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for alphabetizing the props while you were in there.

@elsenhans elsenhans changed the title STCOM-1475 AuditLogModal: accept customItemFormatter prop STCOM-1475 AuditLogModal: accept itemFormatter prop Dec 18, 2025
@elsenhans
Copy link
Contributor

elsenhans commented Dec 18, 2025

@elsenhans Please add itemFormatter the the README.md

@elsenhans
Copy link
Contributor

@zburke Could you please take a look at the changes of the readme.md?

Copy link
Member

@zburke zburke left a comment

Choose a reason for hiding this comment

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

documentation, what even is that? 🫶

Content looks great but please fix the formatting before you merge

Comment on lines 87 to 89
itemFormatter | func |
Formats changed field values of objects or arrays in modal content, used to format oldValue/newValue items of object or array, e.g. showing the field name before the field value.
Receives a field object with `name`, `value`, and `collectionName` properties and the item index. Returns a list item element. | `<li>{field.value}</li>` | false
Copy link
Member

Choose a reason for hiding this comment

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

This needs to all be on one line.

@sonarqubecloud
Copy link

@elsenhans elsenhans merged commit bb948d9 into main Dec 22, 2025
15 checks passed
@elsenhans elsenhans deleted the STCOM-1475-add-custom-item-formatter branch December 22, 2025 12:19
github-actions bot pushed a commit that referenced this pull request Dec 22, 2025
# Description
These changes add the possibility to pass a customItemFormatter to the
AuditLogModal and add to this Formatter the collectionName.

# Why is this change necessary?
While adding additionalCallNumbers to the inventorys item history as
requested per this user story:
https://folio-org.atlassian.net/browse/UIIN-3540 we noticed, that
objects are currently being displayed as unordered, unnamed lists. This
seems to be deficient as the user has to guess which value belongs to
which property.

By adding a custom formatter, we are able to display these objects like
this:

<img width="2978" height="1269" alt="Screenshot from 2025-12-11
15-54-34"
src="https://github.com/user-attachments/assets/8ea804fd-6dcf-4dea-8825-c931c1b9b171"
/>

# Related changes

## Mod-Audit
folio-org/mod-audit#229

## UI-Inventory
folio-org/ui-inventory#2950

---------

Co-authored-by: elsenhans <elsenhans@users.noreply.github.com>
Co-authored-by: elsenhans <48911833+elsenhans@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.

4 participants