URB-3704: Handle an empty list of NOTICe notifications#597
Conversation
📝 WalkthroughWalkthroughThe notice service now handles missing or empty notification data by returning an empty list, and the release notes document the change. ChangesNOTICe notification handling
Estimated code review effort: 2 (Simple) | ~5 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Products/urban/services/notice.py`:
- Around line 138-140: The tests for get_notifications() lack coverage for
responses missing notices or notice. Add a test case in test_notice.py with an
empty or incomplete response and assert get_notifications() returns [] without
raising an error.
🪄 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: 7a2424a6-84fb-4f8d-bdb5-e730d6592c39
📒 Files selected for processing (2)
news/URB-3704.bugfixsrc/Products/urban/services/notice.py
| notices_obj = result.get("notices") or {} | ||
| notices_list = notices_obj.get("notice") or [] | ||
| return notices_list |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for tests exercising the empty/missing notices scenario.
rg -n -C3 'get_notifications' src/Products/urban/services/tests/test_notice.pyRepository: IMIO/Products.urban
Length of output: 474
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== notice.py outline =="
ast-grep outline src/Products/urban/services/notice.py --view expanded || true
echo
echo "== test_notice.py outline =="
ast-grep outline src/Products/urban/services/tests/test_notice.py --view expanded || true
echo
echo "== relevant source lines =="
cat -n src/Products/urban/services/notice.py | sed -n '120,160p'
echo
echo "== relevant test file =="
cat -n src/Products/urban/services/tests/test_notice.pyRepository: IMIO/Products.urban
Length of output: 6268
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== search for notices-related tests =="
rg -n -C2 'notices|notice|get_notifications|_get_notifications' src/Products/urban/services/tests
echo
echo "== search for missing-key or empty-path assertions =="
rg -n -C2 'KeyError|[]|None|empty|missing' src/Products/urban/services/tests/test_notice.pyRepository: IMIO/Products.urban
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path('src/Products/urban/services/notice.py')
text = p.read_text()
for i, line in enumerate(text.splitlines(), 1):
if 'notices_obj' in line or 'notices_list' in line or 'return notices_list' in line:
start = max(1, i-8)
end = min(len(text.splitlines()), i+8)
print(f'-- lines {start}-{end} --')
for j, l in enumerate(text.splitlines()[start-1:end], start):
print(f'{j:4d}: {l}')
print()
PYRepository: IMIO/Products.urban
Length of output: 2752
Add coverage for the empty notices response
src/Products/urban/services/tests/test_notice.py only exercises the populated response path. Add a case where notices/notice is missing and assert get_notifications() returns [] instead of failing on a missing key.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Products/urban/services/notice.py` around lines 138 - 140, The tests for
get_notifications() lack coverage for responses missing notices or notice. Add a
test case in test_notice.py with an empty or incomplete response and assert
get_notifications() returns [] without raising an error.
Only happens to municipalities freshly connected to the NOTICe streams, before receiving their first notification.
Summary by CodeRabbit