Skip to content

Refactor Remarks into a reusable React widget with edit and version history#2959

Open
ramonski wants to merge 12 commits into
2.xfrom
2.x-remarks-react-widget
Open

Refactor Remarks into a reusable React widget with edit and version history#2959
ramonski wants to merge 12 commits into
2.xfrom
2.x-remarks-react-widget

Conversation

@ramonski

@ramonski ramonski commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Description of the issue/feature this PR addresses

Rework the sample Remarks field into a reusable ReactJS widget that lives in senaite.core, served by a unified add/edit/delete/fetch backend that works for both the Archetypes (current samples) and the Dexterity remarks fields. The legacy widget sometimes hung and submitted entries twice, stored remarks as a flat append-only list with no editing, and was wired only through old @@API calls.

Current behavior before PR

  • Remarks on Samples render through a legacy Archetypes skins widget with vanilla JS posting to @@API/update / @@API/read.
  • A delegated click handler (RemarksWidgetView) plus a 10 minute AJAX timeout and no in-flight guard cause occasional hangs and duplicate entries.
  • Remarks are append-only and cannot be edited, sorted, or removed.
  • The newer add_remark / fetch_remarks endpoints only accept the Dexterity field interface, so Samples cannot use them.
  • The remarks field, widget, events and subscriber live under the legacy bika.lims namespace.

Desired behavior after PR is merged

  • A single reusable React widget (comment-thread look with avatars, edit-in-place and collapsible version history) mounts on Samples, Batches and Worksheets today, and on Dexterity content later.
  • Editing a remark updates the visible content, stamps an edited marker and keeps prior versions viewable. Editing is restricted to the author.
  • The manager override is deletion, not editing: LabManager / Manager may soft-delete any remark. The original content is kept for audit, marked as deleted, and a -- deleted by <user> at <time> -- placeholder is shown.
  • A sort-direction toggle (newest/oldest first) lets the user flip the order; the preference is persisted in localStorage and applies to every remarks widget.
  • Submission is hardened against the double-submit and hang: in-flight guard, AbortController, bounded client timeout and a single-record reconcile (no refetch). The legacy RemarksWidgetView binding (the actual double-submit source) and the dead vanilla JS/CSS are removed.
  • Remarks are entered and edited as plain text; the stored sanitized HTML is rendered for display only.
  • Unified backend: the endpoints resolve either field interface; add_remark, edit_remark (author only), delete_remark (manager only) and fetch_remarks (gated by View on the object) each emit a RemarksChangedEvent(context, record, actor, action) so a future message center can notify watchers (no inbox is built here).
  • The remarks field, widget, events and subscriber move to senaite.core, with backward-compatible shims left at the old bika.lims paths.
  • The shared edit form mutation handler is debounced so React re-renders no longer trigger a "Loading" round-trip on every keystroke.

Notes

  • Storage stays backward compatible: the new modified / modified_by / versions / deleted / deleted_by keys are optional and filled lazily, so no GenericSetup upgrade step is required.
  • Includes API_remarks + RemarksField doctests, EN/DE translations and rebuilt webpack bundles.

--
I confirm I have tested this PR thoroughly and coded it according to PEP8
and Plone's Python styleguide standards.

ramonski added 2 commits June 22, 2026 10:30
…istory

Replace the legacy Archetypes remarks widget with a reusable ReactJS widget
served by a unified add/edit/fetch backend that works for both the Archetypes
and the Dexterity remarks fields.

Backend
- New senaite.core.api.remarks: single source of truth for the record shape,
  add/edit, sanitization, localization, permission helpers and the JSON
  data-* attributes consumed by the widget.
- Records gain optional modified / modified_by / versions keys. Editing keeps
  the prior content as a version; no upgrade step is needed (lazy on write).
- Both fields get add()/edit(); the widget endpoints resolve either field
  interface, add an edit_remark endpoint (own-or-manager permission) and emit
  a RemarksChangedEvent(context, record, actor, action) for a future message
  center.
- Move the AT remarks field, widget, events and subscriber to senaite.core,
  keeping BBB shims at the old bika.lims paths.

Frontend
- New React widget under webpack/app/widgets/remarks (comment-thread look with
  avatars, edit-in-place, collapsible version history). Stored HTML is rendered
  for display and a plain-text variant is used in the editor.
- Hardened submission: in-flight guard, AbortController, bounded timeout and a
  single-record reconcile (no refetch) to fix the double-submit and hang.
- Remove the legacy RemarksWidgetView binding (the actual double-submit source)
  and the dead vanilla JS/CSS.
- Debounce the edit form mutation handler so React re-renders no longer trigger
  a "Loading" round-trip on every keystroke.

Templates emit the same mount element in every context (skins widget, z3cform
display-input) with a textarea fallback on the add form.

Includes doctest, EN/DE translations and rebuilt bundles.
Comment thread src/bika/lims/subscribers/remarks.py Fixed
Comment thread src/bika/lims/events/remarks.py Fixed
Comment thread src/bika/lims/events/remarks.py Fixed
Comment thread src/bika/lims/events/remarks.py Fixed
Comment thread src/bika/lims/events/remarks.py Fixed
Comment thread src/bika/lims/browser/fields/remarksfield.py Fixed
Comment thread src/bika/lims/browser/fields/remarksfield.py Fixed
Comment thread src/bika/lims/browser/fields/remarksfield.py Fixed
Comment thread src/bika/lims/browser/widgets/remarkswidget.py Fixed
Comment thread src/senaite/core/browser/fields/remarksfield.py Fixed
- Declare __all__ in the bika.lims BBB shim modules so the intentional
  re-exports are not reported as unused imports
- Simplify a trivial lambda in RemarksHistory.__str__ (map(str, self))
- Translate the widget labels via senaite.core.i18n.translate instead of
  context.translate
- Document why to_plain_text keeps a targeted HTML-to-text converter rather
  than the portal_transforms text/plain transform (which collapses line breaks)

elif isinstance(value, (list, tuple)):
# This is a list, convert to RemarksHistory
remarks = map(lambda item: RemarksHistoryRecord(item), value)
remarks = RemarksHistory([remark, ])
else:
remarks = RemarksHistory(
map(lambda r: RemarksHistoryRecord(r), parsed_remarks))
ramonski added 3 commits June 22, 2026 10:50
- Extend all docstrings in senaite.core.api.remarks with :param: / :returns:
  / :rtype: field markers, matching the style of the other API modules
- Add a dedicated API_remarks doctest covering the helper functions and drop
  the now-duplicated pure-helper assertions from the RemarksField doctest
Move the Avatar component from the remarks widget into the shared
webpack/app/widgets/components directory (next to SearchField etc.) so it can
be reused by other components. Give it its own unscoped Avatar.css and add
optional size / className props; export the get_initials / get_color helpers.
The add and edit endpoints already enforced FieldEditRemarks / manager
permissions, but fetch_remarks was zope.Public without an internal check, so
anyone who knew an object UID could read its remarks. Require View on the
object (context-aware, so clients only see their own samples) and add the
can_view_remarks helper plus doctest coverage for the anonymous case.
@ramonski ramonski requested a review from xispa June 22, 2026 13:34
@ramonski ramonski added the Enhancement ✨ Improvement to existing functionality label Jun 22, 2026
ramonski added 6 commits June 23, 2026 14:51
# Conflicts:
#	src/senaite/core/browser/static/bundles/senaite.core.widgets.css
#	src/senaite/core/subscribers/configure.zcml
- Add a sort direction toggle (newest/oldest first) that persists in
  localStorage and applies to every remarks widget
- Change the manager override from editing to deletion: editing is now
  restricted to the author, while managers may soft-delete any remark
- Deleted remarks are kept for audit but their content is hidden and a
  "-- deleted by <user> at <time> --" placeholder is shown
- Add the delete_remark endpoint, RemarksChangedEvent action "deleted",
  field delete() methods, EN/DE translations and doctest coverage
- Replace the browser confirm() with an inline delete confirmation in the
  remark row (matching the saved-filters bookmark delete UX)
- Hide the content of a deleted remark from display (kept in storage for
  audit) and show only the placeholder
- Centralize the per-record can_edit / can_delete annotation in
  annotate_permissions and use it from all endpoints (the add response now
  also carries can_delete)
- Strip remark content on write for AT/DX parity
- Define ACTION_ADDED / ACTION_EDITED / ACTION_DELETED constants for the
  RemarksChangedEvent subscriber contract
- Extract store_history on the AT field, drop the duplicated permission
  helper in the widget, and minor naming/log/key cleanups
- Managers (incl. Lab Managers) can restore a soft-deleted remark, clearing
  the deletion stamp and making the content visible again
- Add apply_restore / can_restore_record / per-record can_restore flag, the
  field restore() methods, the restore_remark endpoint and the "restored"
  RemarksChangedEvent action
- Show a Restore button on deleted remarks for managers
- Refactor the edit/delete/restore controller handlers onto a shared
  run_reconciling helper
…Clerk)

Deletion and restoration of remarks were gated by ManageSenaite, which is
also granted to LabClerk. Introduce a dedicated `senaite.core: Delete Remarks`
permission granted only to LabManager / Manager, so a LabClerk can add and
edit remarks but cannot delete or restore them.

- Register the permission and grant it in the rolemap
- Route can_delete_record / can_restore_record through can_delete_remarks
- Bump the GenericSetup profile version and add an upgrade step that
  reimports the rolemap to register/grant the permission on existing installs
- Replace the hand-rolled portal_membership.checkPermission calls in the
  permission helpers with bika.lims.api.security.check_permission
- Drop the custom get_user_fullname and use api.get_user_fullname
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Enhancement ✨ Improvement to existing functionality

Development

Successfully merging this pull request may close these issues.

1 participant