Skip to content

ensure that if we have a hash, we do not create a new one as this cau…#51

Merged
gin0115 merged 1 commit intotrunkfrom
fix/gh48-ensure-hash-is-not-recreated-if-set
Oct 22, 2025
Merged

ensure that if we have a hash, we do not create a new one as this cau…#51
gin0115 merged 1 commit intotrunkfrom
fix/gh48-ensure-hash-is-not-recreated-if-set

Conversation

@gin0115
Copy link
Collaborator

@gin0115 gin0115 commented Oct 22, 2025

…ses the attributes to get updated and editor treats as dirty

Changes proposed in this Pull Request

This pull request improves the logic for assigning hashes to date objects in the dateManager function to prevent unnecessary recalculation and to ensure that existing hashes are preserved. The main change is to only generate a new hash if one does not ready exist, and to optionally include the date's id in the hash for uniqueness.

Enhancements to date hash assignment:

  • Updated the dateManager function in event-manager.js to check if a date already has a non-empty hash before generating a new one, preventing overwriting existing hashes. If an id is present and non-empty, it is now included in the hash generation for better uniqueness.

Testing instructions

Mentions #48

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced event date handling to better preserve existing identifiers and improve accuracy when event IDs are available, ensuring more reliable date recognition and processing.

…ses the attributes to get updated and editor treats as dirty

|
@coderabbitai
Copy link

coderabbitai bot commented Oct 22, 2025

Walkthrough

The dateManager initialization logic for hash generation has been modified. Previously, all dates unconditionally received a generated hash via createDateHash(date.start_date, date.end_date). The updated implementation now preserves existing non-empty hashes instead of overwriting them. When generating new hashes for dates without one, the code uses the date's id as a postId parameter if the id is present and non-empty, otherwise falls back to the original method. This makes hash generation conditional on both the presence of existing hashes and the availability of the date's id field.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "ensure that if we have a hash, we do not create a new one as this cau…" directly relates to the main change described in the PR objectives and the code summary. The title accurately captures the core behavioral modification: preserving existing date hashes and preventing them from being overwritten with newly generated hashes. While the title focuses on this primary aspect and doesn't mention the secondary improvement of optionally incorporating the id into hash generation, this is acceptable per the guidelines since PR titles need not cover every detail of the changeset. The title is concise, specific, and clearly communicates the key problem being addressed (issue #48).
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate docstrings

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

@gin0115 gin0115 merged commit f6bc21c into trunk Oct 22, 2025
6 of 7 checks passed
Copy link

@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: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 47e760a and 90363cb.

📒 Files selected for processing (1)
  • src/blocks/event-info/event-manager.js (1 hunks)
🔇 Additional comments (1)
src/blocks/event-info/event-manager.js (1)

59-62: LGTM - preserves existing hashes as intended.

This logic correctly implements the PR objective by skipping hash generation when a non-empty hash already exists.

@@ -56,7 +56,14 @@ const createDateHash = (start, end, postId) => {
export const dateManager = (initialDates = [], timezone = '', metaSync = null) => {
// lOOP through dates and add a hash to each date
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix typo in comment.

The comment has a typo: "lOOP" should be "Loop".

-	// lOOP through dates and add a hash to each date
+	// Loop through dates and add a hash to each date
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// lOOP through dates and add a hash to each date
// Loop through dates and add a hash to each date
🤖 Prompt for AI Agents
In src/blocks/event-info/event-manager.js around line 57, the inline comment
contains a typo ("lOOP through dates and add a hash to each date"); update the
comment to correct casing and spelling to "Loop through dates and add a hash to
each date" so it reads clearly.

Comment on lines +64 to +66
date.hash = date.id && date.id !== ''
? createDateHash(date.start_date, date.end_date, date.id)
: createDateHash(date.start_date, date.end_date);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Inconsistent hash generation across different code paths.

The initialization now conditionally includes date.id in hash generation, but refreshWithNewDates (line 94) and upsertDate (line 240) always call createDateHash without the date.id parameter. This inconsistency means the same date could receive different hash values depending on which code path processes it.

Impact:

  • A date initialized with an id gets a hash like ${start}-${end}-${id}
  • The same date refreshed or upserted gets a hash like ${start}-${end}-${editorPostId}${random}
  • This could cause hash mismatches, duplicate entries, or lost associations

Consider one of these approaches:

  1. Consistent ID-based hashing - Update both refreshWithNewDates and upsertDate to also use date.id when available:
// In refreshWithNewDates (line 94)
date.hash = date.id && date.id !== ''
    ? createDateHash(date.start_date, date.end_date, date.id)
    : createDateHash(date.start_date, date.end_date);
// In upsertDate (line 240)
date.hash = date.id && date.id !== ''
    ? createDateHash(date.start_date, date.end_date, date.id)
    : createDateHash(date.start_date, date.end_date);
  1. Uniform random hashing - Remove the date.id logic from initialization to maintain the original behavior (always use editor post ID + random).

Run the following script to check how these functions are used and whether dates typically have IDs:


🏁 Script executed:

#!/bin/bash
# Description: Check usage patterns of dateManager, refreshWithNewDates, and upsertDate

# Find all calls to dateManager to understand how dates are initialized
echo "=== dateManager initialization calls ==="
rg -n -C3 'dateManager\s*\(' --type=js

# Find all calls to refreshWithNewDates
echo -e "\n=== refreshWithNewDates usage ==="
rg -n -C3 'refreshWithNewDates' --type=js

# Find all calls to upsertDate
echo -e "\n=== upsertDate usage ==="
rg -n -C3 'upsertDate' --type=js

# Look for date object structures to understand when id is set
echo -e "\n=== Date object structures (looking for .id assignments) ==="
rg -n -C2 '\.id\s*[:=]' --type=js -g '!node_modules'

Length of output: 8128


I need to gather more information about the date object structures being used across these different code paths.

Let me search the codebase more deeply to understand the date object structures.

[/run_scripts]


🏁 Script executed:

#!/bin/bash
# Look for createDefaultDate implementation
echo "=== createDefaultDate function ==="
rg -n -A10 'const createDefaultDate|function createDefaultDate' --type=js

# Look for where eventDatePosts is built/passed from
echo -e "\n=== eventDatePosts source ==="
rg -n -B5 -A5 'eventDatePosts' --type=js

# Look for any date.id assignments in the codebase
echo -e "\n=== Checking if date.id is set anywhere ==="
rg -n '\$.*\.id\s*=|date\.id\s*=' --type=js

# Look at the main block/component that initializes dateManager
echo -e "\n=== index.js dateManager initialization context ==="
rg -n -B15 'dateManager\(eventDatePosts' --type=js

Length of output: 3596


I need to examine the getEventDatePosts function and understand what date structure it returns.

[/run_scripts]


🏁 Script executed:

#!/bin/bash
# Find getEventDatePosts implementation
echo "=== getEventDatePosts function ==="
rg -n -A20 'const getEventDatePosts|function getEventDatePosts' --type=js

# Look for where response.dates comes from in refreshWithNewDates call
echo -e "\n=== API response context ==="
rg -n -B10 'response\.dates' --type=js | head -50

# Check for any REST API endpoints that provide dates
echo -e "\n=== API endpoint definitions ==="
rg -n 'wp\.apiFetch|fetch\(' --type=js | grep -i date | head -20

Length of output: 2204


Let me examine how hashes are used in the codebase and whether this inconsistency would cause actual issues.

[/run_scripts]


🏁 Script executed:

#!/bin/bash
# Search for how hash is used - lookups, comparisons, etc.
echo "=== Hash usage patterns ==="
rg -n '\.hash' --type=js -C2

# Look for date finding/matching logic
echo -e "\n=== Date lookup/matching logic ==="
rg -n 'find.*hash|filter.*hash' --type=js -C2

# Check removeDate implementation
echo -e "\n=== removeDate implementation ==="
rg -n -A15 'const removeDate|function removeDate' --type=js | head -40

Length of output: 5738


Let me check the createDateHash implementation and understand if date.id from backend would actually cause problems.

[/run_scripts]


🏁 Script executed:

#!/bin/bash
# Look at createDateHash implementation
echo "=== createDateHash function ==="
rg -n -A10 'const createDateHash|function createDateHash' --type=js

# Check if there's any documentation about date.id
echo -e "\n=== Date structure documentation ==="
rg -n 'date\.id|postId|@param.*date' --type=js | grep -i -E 'param|id|structure' | head -20

Length of output: 3512


Update lines 94 and 240 in refreshWithNewDates and upsertDate to conditionally use date.id when generating hashes, matching the initialization logic at lines 64-66.

The hash is the unique identifier used throughout the manager for finding, updating, and removing dates. The initialization path conditionally includes date.id in hash generation (lines 64-66), but refreshWithNewDates (line 94) and upsertDate (line 240) ignore it entirely. Since date objects are documented to contain an id field (DateTimeGroup.js line 37), this creates hash mismatches:

  • Initial load: hash = "${start}-${end}-${dateId}" (when id exists)
  • On refresh/upsert: hash = "${start}-${end}-${editorPostId}${random}" (ignores id)

This causes upsertDate to fail finding existing dates by hash (line 244), creating duplicates instead of updating. Similarly, removeDate cannot locate dates that were initialized with an id.

Fix: Apply the same conditional logic from lines 64-66 to both line 94 and line 240:

// In refreshWithNewDates (line 94)
date.hash = date.id && date.id !== ''
    ? createDateHash(date.start_date, date.end_date, date.id)
    : createDateHash(date.start_date, date.end_date);
// In upsertDate (line 240)
date.hash = date.id && date.id !== ''
    ? createDateHash(date.start_date, date.end_date, date.id)
    : createDateHash(date.start_date, date.end_date);
🤖 Prompt for AI Agents
In src/blocks/event-info/event-manager.js (context: lines ~64-66, 94, 240), the
initialization sets date.hash conditionally including date.id when present, but
refreshWithNewDates (line ~94) and upsertDate (line ~240) always generate the
hash without date.id causing mismatches and duplicate entries; update both
places to use the same conditional logic as lines 64-66: compute date.hash using
createDateHash(date.start_date, date.end_date, date.id) only when date.id exists
and is non-empty, otherwise call createDateHash with just start and end dates so
hash generation is consistent across initialization, refresh, and upsert.

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.

1 participant