Skip to content

Conversation

@devksingh4
Copy link
Member

@devksingh4 devksingh4 commented Nov 8, 2025

Fixes #387

Summary by CodeRabbit

  • Refactor
    • Improved internal handling of metadata fields in the event editor for more stable creation, editing, and removal—no change to visible behavior or limits.
  • Tests
    • Added end-to-end coverage for event lifecycle: creating an event (including dynamic key/value fields) and deleting it, using a test-scoped identifier to validate flow.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 8, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Changed metadata handling in ManageEvent.page: introduced an initialization guard and switched from key-based to id-based metadata entries (stable ids like meta-<timestamp>-<random>). Updated all metadata operations and UI bindings to use entry IDs; adjusted effect dependency to form.values.metadata. Added/renamed e2e tests for event lifecycle.

Changes

Cohort / File(s) Summary
Metadata ID-based sync refactor
src/ui/pages/events/ManageEvent.page.tsx
Introduced initializedRef to avoid re-sync after initial load. Initialized metadataEntries once from form.values.metadata using stable ids (meta-<timestamp>-<random>). Reworked metadata handlers: addMetadataField, updateMetadataValue(entryId, ...), updateMetadataKey(entryId, ...), removeMetadataField(entryId). UI bindings now pass entryId for key/value/delete. useEffect dependency changed to form.values.metadata.
E2E tests — event lifecycle
tests/e2e/events.spec.ts
Renamed top-level suite and added a serial "Event lifecycle test" suite with two tests: create event (fills form including dynamic metadata fields) and delete event (finds created event by testId, confirms deletion). Introduced testId constant shared across tests.

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant Component as ManageEvent Component
    participant Form as Form State
    participant Metadata as metadataEntries (state)

    Note over Component: Initial load
    Component->>Form: read form.values.metadata
    Form-->>Component: metadata object
    Component->>Component: generate stable ids (meta-<ts>-<rand>)
    Component->>Metadata: set metadataEntries (id -> {key, value})
    Component->>Component: set initializedRef = true

    Note over Component: Edit key
    User->>Component: change key input (entryId)
    Component->>Metadata: updateMetadataKey(entryId, newKey)
    Metadata->>Form: remove oldKey, add newKey with value

    Note over Component: Edit value
    User->>Component: change value input (entryId)
    Component->>Metadata: updateMetadataValue(entryId, newValue)
    Metadata->>Form: set form.values.metadata[key] = newValue

    Note over Component: Remove field
    User->>Component: click delete (entryId)
    Component->>Metadata: removeMetadataField(entryId)
    Metadata->>Form: delete key from form.values.metadata

    Note over Component: Subsequent changes
    Component->>Component: useEffect runs on form.values.metadata changes
    Component-->>Component: skip re-initialization if initializedRef == true
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Areas needing attention:
    • src/ui/pages/events/ManageEvent.page.tsx: verify ID generation approach avoids collisions and is stable across the component lifecycle.
    • Confirm initializedRef reliably prevents re-initialization without masking legitimate updates.
    • Ensure bidirectional sync correctness between metadataEntries and form.values.metadata for all handlers (add/update key/update value/remove).
    • UI bindings: confirm every interactive control uses entryId and that events correctly identify the targeted entry.
    • tests/e2e/events.spec.ts: validate the added create/delete tests are deterministic (testId usage) and cleanup/isolation assumptions.

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Fix event management metadata UI' accurately reflects the main change—addressing metadata UI issues in event management.
Linked Issues check ✅ Passed The changes address issue #387 requirements: metadata synchronization via IDs prevents unwanted re-validation on onChange, and ID-based tracking enables proper field editing and value pasting.
Out of Scope Changes check ✅ Passed All changes remain focused on fixing metadata field input, synchronization, and event management—directly addressing the linked issue without extraneous modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between 8de317b and 08f4ed2.

📒 Files selected for processing (1)
  • tests/e2e/events.spec.ts (2 hunks)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 8, 2025

💰 Infracost report

Monthly estimate generated

This comment will be updated when code changes.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between 220eef4 and 8de317b.

📒 Files selected for processing (1)
  • src/ui/pages/events/ManageEvent.page.tsx (5 hunks)
🧰 Additional context used
🪛 GitHub Actions: Required Reviews
src/ui/pages/events/ManageEvent.page.tsx

[error] 1-1: Requirement "Base Requirement" is not satisfied by the existing reviews.

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Run Unit Tests
  • GitHub Check: Build Application

@devksingh4 devksingh4 merged commit 9e7e177 into main Nov 8, 2025
7 of 9 checks passed
@devksingh4 devksingh4 deleted the dsingh14/fix-event-metadata branch November 8, 2025 23:44
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.

Event metadata cannot be edited

2 participants