fix: ensure getDecisionDate indexer always returns DateTime - #546
Open
WBoudabous wants to merge 2 commits into
Open
fix: ensure getDecisionDate indexer always returns DateTime#546WBoudabous wants to merge 2 commits into
WBoudabous wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/CHANGES.rst (1)
13-20: Avoid duplicateBug fixes:subsections for the same release entry.Under
2.9.17, this adds a secondBug fixes:block. Please merge both items into one block (or move to a news fragment if your release process enforces that).Proposed cleanup
-Bug fixes: - -- Fix decisionDate filter: ensure getDecisionDate indexer always -returns a DateTime (including PloneMeeting data). -[WBoudabous] (SUP-52501) - - -Bug fixes: +Bug fixes: + +- Fix decisionDate filter: ensure getDecisionDate indexer always + returns a DateTime (including PloneMeeting data). + [WBoudabous] (SUP-52501) - Fix missing import [jchandelle] (SUP-52327)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/CHANGES.rst` around lines 13 - 20, The CHANGES.rst entry for release 2.9.17 contains a duplicated "Bug fixes:" subsection; consolidate them by merging the two "Bug fixes:" blocks into a single subsection (combine the line mentioning "Fix decisionDate filter: ensure getDecisionDate indexer always returns a DateTime (including PloneMeeting data)." with the existing Bug fixes list) or, if your release workflow requires, move the new item into an appropriate news fragment instead; ensure the final 2.9.17 section has only one "Bug fixes:" header and includes the getDecisionDate indexer note.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Products/urban/indexes.py`:
- Around line 277-280: The fallback path still returns a plain datetime.date and
doesn't safely access the PloneMeeting data; update the logic in the function
that builds the index value (the block referencing linked_pm_items and calling
decision_event.getDecisionDate()/getEventDate()) to: 1) guard access to
linked_pm_items[0]["extra_include_meeting"] by using
.get("extra_include_meeting") and then .get("date") to avoid
KeyError/AttributeError; 2) when a date string is present create and return a
DateTime(date_str); and 3) for the existing fallbacks call
decision_event.getDecisionDate() and decision_event.getEventDate() and, if they
are a datetime.date or datetime.datetime, convert them to a DateTime instance
before returning (use the DateTime constructor), so the function always returns
a DateTime. Ensure DateTime is imported where used.
---
Nitpick comments:
In `@docs/CHANGES.rst`:
- Around line 13-20: The CHANGES.rst entry for release 2.9.17 contains a
duplicated "Bug fixes:" subsection; consolidate them by merging the two "Bug
fixes:" blocks into a single subsection (combine the line mentioning "Fix
decisionDate filter: ensure getDecisionDate indexer always returns a DateTime
(including PloneMeeting data)." with the existing Bug fixes list) or, if your
release workflow requires, move the new item into an appropriate news fragment
instead; ensure the final 2.9.17 section has only one "Bug fixes:" header and
includes the getDecisionDate indexer note.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 11a09c01-7a3d-45fa-9436-4e76ba875461
📒 Files selected for processing (2)
docs/CHANGES.rstsrc/Products/urban/indexes.py
Comment on lines
+277
to
280
| date_str = linked_pm_items[0]["extra_include_meeting"].get("date") | ||
| if date_str: | ||
| return DateTime(date_str) | ||
| return decision_event.getDecisionDate() or decision_event.getEventDate() |
There was a problem hiding this comment.
getDecisionDate is still not guaranteed to return DateTime in all paths.
The new PloneMeeting path is improved, but the fallback at Line [263]-Line [275] still returns datetime.date, which violates the PR objective and can conflict with DateIndex. Also, linked_pm_items[0]["extra_include_meeting"] is still not safely guarded.
Proposed fix
@@
- if brain and brain[0].getDecisionDate:
- old_decision_date = brain[0].getDecisionDate
- if type(old_decision_date) is DateTime:
- decision_date = date(
- old_decision_date.year(),
- old_decision_date.month(),
- old_decision_date.day(),
- )
- else:
- decision_date = date(
- old_decision_date.year,
- old_decision_date.month,
- old_decision_date.day,
- )
- return decision_date
+ if brain and brain[0].getDecisionDate:
+ old_decision_date = brain[0].getDecisionDate
+ if isinstance(old_decision_date, DateTime):
+ return old_decision_date
+ return DateTime(old_decision_date.isoformat())
if linked_pm_items:
- date_str = linked_pm_items[0]["extra_include_meeting"].get("date")
+ meeting = linked_pm_items[0].get("extra_include_meeting") or {}
+ date_str = meeting.get("date")
if date_str:
return DateTime(date_str)
- return decision_event.getDecisionDate() or decision_event.getEventDate()
+ decision_date = decision_event.getDecisionDate() or decision_event.getEventDate()
+ if decision_date and not isinstance(decision_date, DateTime):
+ decision_date = DateTime(decision_date.isoformat())
+ return decision_date🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Products/urban/indexes.py` around lines 277 - 280, The fallback path
still returns a plain datetime.date and doesn't safely access the PloneMeeting
data; update the logic in the function that builds the index value (the block
referencing linked_pm_items and calling
decision_event.getDecisionDate()/getEventDate()) to: 1) guard access to
linked_pm_items[0]["extra_include_meeting"] by using
.get("extra_include_meeting") and then .get("date") to avoid
KeyError/AttributeError; 2) when a date string is present create and return a
DateTime(date_str); and 3) for the existing fallbacks call
decision_event.getDecisionDate() and decision_event.getEventDate() and, if they
are a datetime.date or datetime.datetime, convert them to a DateTime instance
before returning (use the DateTime constructor), so the function always returns
a DateTime. Ensure DateTime is imported where used.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR ensures that the getDecisionDate indexer always returns a DateTime.
Summary by CodeRabbit
Release Notes