[change] Added default ordering to timestamped models - #728
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (6)**/*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*📄 CodeRabbit inference engine (Custom checks)
Files:
⚙️ CodeRabbit configuration file
Files:
**/tests/**/*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*tests*/**⚙️ CodeRabbit configuration file
Files:
**/*.{md,rst,txt}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{md,rst}⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (1)📚 Learning: 2026-06-15T22:45:01.529ZApplied to files:
🔇 Additional comments (9)
📝 WalkthroughWalkthrough
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (8 files)
Previous Review Summaries (3 snapshots, latest commit a121fe3)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit a121fe3)Status: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
Previous review (commit 594c9ce)Status: No Issues Found | Recommendation: Merge Files Reviewed (6 files)
Previous review (commit 54bfd28)Status: No Issues Found | Recommendation: Merge Files Reviewed (7 files)
Reviewed by step-3.7-flash · Input: 123.5K · Output: 21.7K · Cached: 935.8K |
![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
🤖 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 `@openwisp_utils/metric_collection/tests/test_models.py`:
- Around line 25-27: Extend test_consent_ordering in
openwisp_utils/metric_collection/tests/test_models.py#L25-L27 with deterministic
Consent records using controlled created values, then assert the default
queryset returns newest records first. In
tests/test_project/tests/test_model.py#L16-L18, add deterministic Shelf and Book
records and assert default queryset ordering, QuerySet.order_by() behavior, and
explicit Meta.ordering overrides; retain the existing metadata checks where
applicable.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a4440db8-175b-42a5-9b3f-5c8cfe8d7e38
📒 Files selected for processing (7)
openwisp_utils/base.pyopenwisp_utils/metric_collection/migrations/0002_alter_consent_options.pyopenwisp_utils/metric_collection/tests/test_models.pytests/test_project/migrations/0010_alter_book_options_alter_shelf_options.pytests/test_project/models.pytests/test_project/tests/test_model.pytests/test_project/tests/test_selenium.py
📜 Review details
⏰ Context from checks skipped due to timeout. (18)
- GitHub Check: Kilo Code Review
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Analyze (actions)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Place imports at the top of the file; only defer imports when necessary (e.g., Django model imports inside functions or methods where the app registry is not yet ready)
Avoid unnecessary blank lines inside function and method bodies
Add or update tests for every behavior change
Runopenwisp-qa-formatafter editing
Prefer in-process tests so coverage tools can measure changed code
When checking coverage for a changed module, usepython -m pytest <test_path> --cov=<dotted.module.path> --cov-report=term-missing
Watch for unsafe file paths, unsafe subprocess usage, token or secret exposure, and changes that could weaken QA or release safeguards
Write comments and docstrings only when they explain why code is shaped a certain way; place comments before the relevant code block instead of scattering them inside itIn Django code, mark all user-facing strings as translatable using the Django internationalization framework.
Files:
tests/test_project/tests/test_model.pytests/test_project/migrations/0010_alter_book_options_alter_shelf_options.pyopenwisp_utils/metric_collection/migrations/0002_alter_consent_options.pytests/test_project/tests/test_selenium.pyopenwisp_utils/base.pytests/test_project/models.pyopenwisp_utils/metric_collection/tests/test_models.py
**/tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
For bug fixes, write the regression test first, run it against the unfixed code, confirm it fails for the expected reason, then implement the fix
Files:
tests/test_project/tests/test_model.pytests/test_project/migrations/0010_alter_book_options_alter_shelf_options.pytests/test_project/tests/test_selenium.pytests/test_project/models.pyopenwisp_utils/metric_collection/tests/test_models.py
**/*
📄 CodeRabbit inference engine (Custom checks)
**/*: For bug fixes in core user-facing functionality, fix the root cause rather than hiding or bypassing the problem.
Bug fixes must include a regression test that reproduces the bug and fails when the fix is removed, except for valid exceptions such as one-line fixes or impractical GitHub Actions workflow fixes.
User-interface bug fixes must include a Selenium browser regression test; missing coverage should be reported as a warning.
Tests must be deterministic and non-flaky; avoid dependence on timing, sleeps, system time, randomness without a fixed seed, race conditions, network services, filesystem state, environment-specific configuration, execution order, shared global state, hardcoded ports, or improperly awaited asynchronous operations.
When a change request modifies documented behavior, update the documentation to describe the new behavior.
Pull requests containing UI changes must include screenshots or screen recordings in the pull request description.
Update tests for non-trivial behavior changes and validate the modified behavior.
Flag changes that may break existing behavior or public APIs unless a clear migration path is provided.
Feature requests must have an issue explaining the feature, validated and accepted by an organization member through labels or project assignment.
New features must be documented; heavily UI-related features should have a dedicated documentation section or page.
Add tests for new features without significantly decreasing coverage; prefer Selenium browser tests for UI-impacting features.
Non-organization members must target maintainer-validated issues with at least one label other thanwontfixorinvalidand assignment to either the OpenWISP Contributor's Board or OpenWISP Priorities for next releases project.
Flag potential security vulnerabilities.
Flag obvious performance regressions such as heavy loops, repeated I/O, or unoptimized queries.
Cryptic or non-obvious code, including regexes, complex shell commands, and ...
Files:
tests/test_project/tests/test_model.pytests/test_project/migrations/0010_alter_book_options_alter_shelf_options.pyopenwisp_utils/metric_collection/migrations/0002_alter_consent_options.pytests/test_project/tests/test_selenium.pyopenwisp_utils/base.pytests/test_project/models.pyopenwisp_utils/metric_collection/tests/test_models.py
🧠 Learnings (1)
📚 Learning: 2026-06-15T22:45:01.529Z
Learnt from: nemesifier
Repo: openwisp/openwisp-utils PR: 701
File: setup.py:72-72
Timestamp: 2026-06-15T22:45:01.529Z
Learning: When reviewing changes that require a minimum Selenium Python version for specific APIs (e.g., BiDi features like `Options.enable_bidi`, console/message handlers like `Script.add_console_message_handler`, or event handlers like `BrowsingContext.add_event_handler`), do not use the Selenium API docs site (`https://www.selenium.dev/selenium/docs/api/py/`) as the source of introduction/version history, since it reflects the latest API shape. Instead, confirm feature availability by checking the official Python changelog (`py/CHANGES` in the Selenium repo) and/or inspecting the Selenium source (tags/commits corresponding to candidate versions). Only accept the stated minimum version after verifying that the referenced APIs exist in that Selenium version (e.g., Selenium 4.32.0 includes the BiDi APIs used by openwisp-utils’ BiDi-related PR `#701` as of that version).
Applied to files:
tests/test_project/tests/test_model.pytests/test_project/migrations/0010_alter_book_options_alter_shelf_options.pyopenwisp_utils/metric_collection/migrations/0002_alter_consent_options.pytests/test_project/tests/test_selenium.pyopenwisp_utils/base.pytests/test_project/models.pyopenwisp_utils/metric_collection/tests/test_models.py
🔇 Additional comments (5)
tests/test_project/models.py (1)
59-60: LGTM!Also applies to: 77-78
openwisp_utils/metric_collection/migrations/0002_alter_consent_options.py (1)
1-17: LGTM!tests/test_project/migrations/0010_alter_book_options_alter_shelf_options.py (1)
1-21: LGTM!tests/test_project/tests/test_selenium.py (1)
655-658: LGTM!openwisp_utils/base.py (1)
26-26: 🗄️ Data Integrity & IntegrationShared ordering is already preserved.
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 `@tests/test_project/tests/test_model.py`:
- Around line 58-71: Extend the ordering test around Shelf and Book to cover a
timestamped descendant with an explicit Meta.ordering override. Assert both that
the model’s _meta.ordering equals its explicit ordering and that querying it
returns records in that declared order, using deterministic fixtures and
primary-key tie-breaking where needed.
🪄 Autofix
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 40152959-4556-47f9-9305-7a98f4ef7d1f
📒 Files selected for processing (6)
docs/developer/other-utilities.rstopenwisp_utils/base.pyopenwisp_utils/metric_collection/migrations/0002_alter_consent_options.pyopenwisp_utils/metric_collection/tests/test_models.pytests/test_project/migrations/0010_alter_book_options_alter_shelf_options.pytests/test_project/tests/test_model.py
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Kilo Code Review
🧰 Additional context used
📓 Path-based instructions (6)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Place imports at the top of the file; only defer imports when necessary (e.g., Django model imports inside functions or methods where the app registry is not yet ready)
Avoid unnecessary blank lines inside function and method bodies
Add or update tests for every behavior change
Runopenwisp-qa-formatafter editing
Prefer in-process tests so coverage tools can measure changed code
When checking coverage for a changed module, usepython -m pytest <test_path> --cov=<dotted.module.path> --cov-report=term-missing
Watch for unsafe file paths, unsafe subprocess usage, token or secret exposure, and changes that could weaken QA or release safeguards
Write comments and docstrings only when they explain why code is shaped a certain way; place comments before the relevant code block instead of scattering them inside itIn Django code, mark all user-facing strings as translatable using the Django internationalization framework.
Files:
openwisp_utils/metric_collection/tests/test_models.pytests/test_project/tests/test_model.pyopenwisp_utils/base.pyopenwisp_utils/metric_collection/migrations/0002_alter_consent_options.pytests/test_project/migrations/0010_alter_book_options_alter_shelf_options.py
**/tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
For bug fixes, write the regression test first, run it against the unfixed code, confirm it fails for the expected reason, then implement the fix
Files:
openwisp_utils/metric_collection/tests/test_models.pytests/test_project/tests/test_model.pytests/test_project/migrations/0010_alter_book_options_alter_shelf_options.py
**/*
📄 CodeRabbit inference engine (Custom checks)
**/*: For bug fixes in core user-facing functionality, fix the root cause rather than hiding or bypassing the problem.
Bug fixes must include a regression test that reproduces the bug and fails when the fix is removed, except for valid exceptions such as one-line fixes or impractical GitHub Actions workflow fixes.
User-interface bug fixes must include a Selenium browser regression test; missing coverage should be reported as a warning.
Tests must be deterministic and non-flaky; avoid dependence on timing, sleeps, system time, randomness without a fixed seed, race conditions, network services, filesystem state, environment-specific configuration, execution order, shared global state, hardcoded ports, or improperly awaited asynchronous operations.
When a change request modifies documented behavior, update the documentation to describe the new behavior.
Pull requests containing UI changes must include screenshots or screen recordings in the pull request description.
Update tests for non-trivial behavior changes and validate the modified behavior.
Flag changes that may break existing behavior or public APIs unless a clear migration path is provided.
Feature requests must have an issue explaining the feature, validated and accepted by an organization member through labels or project assignment.
New features must be documented; heavily UI-related features should have a dedicated documentation section or page.
Add tests for new features without significantly decreasing coverage; prefer Selenium browser tests for UI-impacting features.
Non-organization members must target maintainer-validated issues with at least one label other thanwontfixorinvalidand assignment to either the OpenWISP Contributor's Board or OpenWISP Priorities for next releases project.
Flag potential security vulnerabilities.
Flag obvious performance regressions such as heavy loops, repeated I/O, or unoptimized queries.
Cryptic or non-obvious code, including regexes, complex shell commands, and ...
Files:
openwisp_utils/metric_collection/tests/test_models.pytests/test_project/tests/test_model.pyopenwisp_utils/base.pydocs/developer/other-utilities.rstopenwisp_utils/metric_collection/migrations/0002_alter_consent_options.pytests/test_project/migrations/0010_alter_book_options_alter_shelf_options.py
⚙️ CodeRabbit configuration file
**/*: - Flag potential security vulnerabilities
Flag obvious performance regressions, such as heavy loops, repeated I/O, or unoptimized queries
Flag unused or redundant code
Flag outdated or incorrect comments/docstrings
Ensure new code handles errors properly:
- Log errors that cannot be resolved by the user with error level
- Log unusual conditions with warning level
- Log important background actions with info level
- Provide user-facing messages for errors that the user can solve autonomously (for example, validation errors)
Files:
openwisp_utils/metric_collection/tests/test_models.pytests/test_project/tests/test_model.pyopenwisp_utils/base.pydocs/developer/other-utilities.rstopenwisp_utils/metric_collection/migrations/0002_alter_consent_options.pytests/test_project/migrations/0010_alter_book_options_alter_shelf_options.py
**/*tests*/**
⚙️ CodeRabbit configuration file
**/*tests*/**: Ensure tests cover relevant success, error, boundary, and unusual
input scenarios.Flag tests that depend on arbitrary sleeps, uncontrolled system time,
specific timezones, unseeded randomness, network access, external
services, execution order, shared mutable state, hardcoded ports, or
asynchronous operations that are not properly awaited.
Files:
openwisp_utils/metric_collection/tests/test_models.pytests/test_project/tests/test_model.pytests/test_project/migrations/0010_alter_book_options_alter_shelf_options.py
**/*.{md,rst,txt}
📄 CodeRabbit inference engine (AGENTS.md)
Update documentation when behavior, settings, public APIs, setup steps, QA rules, or supported versions change
Files:
docs/developer/other-utilities.rst
**/*.{md,rst}
⚙️ CodeRabbit configuration file
**/*.{md,rst}: Verify that documentation remains consistent with the implemented
behavior and does not reference deprecated or removed functionality.
Files:
docs/developer/other-utilities.rst
🧠 Learnings (1)
📚 Learning: 2026-06-15T22:45:01.529Z
Learnt from: nemesifier
Repo: openwisp/openwisp-utils PR: 701
File: setup.py:72-72
Timestamp: 2026-06-15T22:45:01.529Z
Learning: When reviewing changes that require a minimum Selenium Python version for specific APIs (e.g., BiDi features like `Options.enable_bidi`, console/message handlers like `Script.add_console_message_handler`, or event handlers like `BrowsingContext.add_event_handler`), do not use the Selenium API docs site (`https://www.selenium.dev/selenium/docs/api/py/`) as the source of introduction/version history, since it reflects the latest API shape. Instead, confirm feature availability by checking the official Python changelog (`py/CHANGES` in the Selenium repo) and/or inspecting the Selenium source (tags/commits corresponding to candidate versions). Only accept the stated minimum version after verifying that the referenced APIs exist in that Selenium version (e.g., Selenium 4.32.0 includes the BiDi APIs used by openwisp-utils’ BiDi-related PR `#701` as of that version).
Applied to files:
openwisp_utils/metric_collection/tests/test_models.pytests/test_project/tests/test_model.pyopenwisp_utils/base.pyopenwisp_utils/metric_collection/migrations/0002_alter_consent_options.pytests/test_project/migrations/0010_alter_book_options_alter_shelf_options.py
🔇 Additional comments (5)
openwisp_utils/base.py (1)
26-26: LGTM!docs/developer/other-utilities.rst (1)
30-34: LGTM!openwisp_utils/metric_collection/migrations/0002_alter_consent_options.py (1)
13-16: LGTM!tests/test_project/migrations/0010_alter_book_options_alter_shelf_options.py (1)
13-20: LGTM!openwisp_utils/metric_collection/tests/test_models.py (1)
1-3: LGTM!Also applies to: 26-42
a121fe3 to
dc0b2ef
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
@coderabbitai resolve |
✅ Action performedComments resolved and changes approved. |
1 participant
Checklist
Reference to Existing Issue
Closes #727.
Description of Changes
Adds deterministic newest-first default ordering,
("-created", "-pk"), toTimeStampedEditableModeland preserves it in local test models that declareMeta.Adds metadata-only migrations, behavioral queryset coverage, and developer documentation. The autocomplete filter test selects the expected shelf by label instead of relying on database result order.
Screenshot
N/A