Skip to content

[chores:fix] Add ApiKey proxy model and permissions to sample users#228

Merged
nemesifier merged 2 commits into
masterfrom
fix-apikey-and-ci
Jul 1, 2026
Merged

[chores:fix] Add ApiKey proxy model and permissions to sample users#228
nemesifier merged 2 commits into
masterfrom
fix-apikey-and-ci

Conversation

@nemesifier

@nemesifier nemesifier commented Jul 1, 2026

Copy link
Copy Markdown
Member

Checklist

  • I have read the OpenWISP Contributing Guidelines.
  • I have manually tested the changes proposed in this pull request.
  • I have written new test cases for new code and/or updated existing tests for changes to existing code.
  • I have updated the documentation.

Reference to Existing Issue

No existing issue. Replicates openwisp/openwisp-controller@9ca18cc for the IPAM sample app.

Description of Changes

Adds the ApiKey proxy model to the sample users test app migrations and model registration.

Grants API key permissions to sample users admin groups during default group setup.

Adds the authtoken migration dependency required by the ApiKey proxy model.

Screenshot

Not applicable. This is a test migration change.

@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: fa3bc171-d06a-4b5a-aa25-25c96bc1a937

📥 Commits

Reviewing files that changed from the base of the PR and between 522dd6b and f1032de.

📒 Files selected for processing (2)
  • tests/openwisp2/sample_users/migrations/0003_user_expiration_date_user_user_active_expiry_idx.py
  • tests/openwisp2/sample_users/models.py
📜 Recent review details
⏰ Context from checks skipped due to timeout. (14)
  • GitHub Check: Python==3.13 | django~=5.2.0
  • GitHub Check: Python==3.13 | django~=5.1.0
  • GitHub Check: Python==3.10 | django~=4.2.0
  • GitHub Check: Python==3.11 | django~=4.2.0
  • GitHub Check: Python==3.12 | django~=4.2.0
  • GitHub Check: Python==3.10 | django~=5.1.0
  • GitHub Check: Python==3.12 | django~=5.1.0
  • GitHub Check: Python==3.10 | django~=5.2.0
  • GitHub Check: Python==3.11 | django~=5.1.0
  • GitHub Check: Python==3.12 | django~=5.2.0
  • GitHub Check: Python==3.11 | django~=5.2.0
  • GitHub Check: Analyze (actions)
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{py,html}

📄 CodeRabbit inference engine (Custom checks)

For Django pull requests, ensure all user-facing strings are marked as translatable using the Django i18n framework

Files:

  • tests/openwisp2/sample_users/models.py
  • tests/openwisp2/sample_users/migrations/0003_user_expiration_date_user_user_active_expiry_idx.py
**/*.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.
Mark user-facing strings for translation with Django i18n helpers in Django code.
Write comments and docstrings only when they explain why code is shaped a certain way. Put comments before the relevant code block instead of scattering them inside it.

Files:

  • tests/openwisp2/sample_users/models.py
  • tests/openwisp2/sample_users/migrations/0003_user_expiration_date_user_user_active_expiry_idx.py
🧠 Learnings (1)
📚 Learning: 2026-06-01T21:59:25.519Z
Learnt from: nemesifier
Repo: openwisp/openwisp-ipam PR: 218
File: tests/openwisp2/sample_users/migrations/0003_user_expiration_date_user_user_active_expiry_idx.py:17-21
Timestamp: 2026-06-01T21:59:25.519Z
Learning: In openwisp-ipam test project migration files under tests/openwisp2/sample_users/migrations/, do not wrap model field `verbose_name` and `help_text` strings with Django’s i18n helpers (e.g., `gettext_lazy`). Keep these strings as plain literals in these migrations; reviewers should not suggest adding translation wrappers for these test migration strings.

Applied to files:

  • tests/openwisp2/sample_users/migrations/0003_user_expiration_date_user_user_active_expiry_idx.py
🔇 Additional comments (3)
tests/openwisp2/sample_users/models.py (1)

52-52: 📐 Maintainability & Code Quality

Confirm the deferred ApiKey import is necessary.

If this isn't breaking a circular dependency, move ApiKey back into the existing top-level import block so the module follows the repo's import-order rule.

As per coding guidelines: **/*.py imports should live at the top unless deferral is necessary.

Source: Coding guidelines

tests/openwisp2/sample_users/migrations/0003_user_expiration_date_user_user_active_expiry_idx.py (2)

8-12: Missing authtoken dependency now added.

The previously flagged missing rest_framework.authtoken migration dependency has been addressed with the ("authtoken", "0001_initial") entry.


25-36: LGTM!


📝 Walkthrough

Walkthrough

Test migrations for the sample_users app were extended: one migration now runs a data migration to add API key permissions to admins, and another adds a CreateModel operation defining an ApiKey proxy model over authtoken.token. The models.py file was updated to import ApiKey from openwisp_users.base.models.

Estimated code review effort: 1 (Trivial) | ~5 minutes


Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

  • Ignore

❌ Failed checks (2 errors, 3 warnings)

Check name Status Explanation Resolution
Bug Fixes ❌ Error No regression test was added; the patch only updates migrations/models, and I found no test exercising the ApiKey permission/setup path. Add a deterministic test that boots sample_users migrations and asserts ApiKey permissions/default groups, so removing the patch fails it.
General Rules ❌ Error tests/openwisp2/sample_users/models.py adds ApiKey at the bottom, violating AGENTS.md’s import-placement rule; no tests cover the new ApiKey path. Move the ApiKey import to the top of models.py (or justify the defer), and add tests covering the new sample_users migration/model registration behavior.
Title check ⚠️ Warning The title is descriptive but uses an invalid prefix, "[chores:fix]", instead of one allowed type. Change the prefix to a single allowed type such as [fix] or [chores] and keep the rest of the title descriptive.
Features ⚠️ Warning Missing issue/validation, docs update, and new tests; the PR description says “No existing issue” and leaves docs/tests unchecked. Add a linked/accepted issue, document the ApiKey feature, and add tests for the migration/admin permission behavior; screenshots aren’t needed.
Changes ⚠️ Warning No tests were added/updated for the new ApiKey model and permission migration; tests/openwisp2/sample_users/tests.py is unchanged and doesn't cover it. Add integration tests for sample_users migrations asserting ApiKey registration and admin-group API-key permissions after 0002/0003 apply.
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed The description matches the template sections and includes checklist, issue context, changes, and screenshot status.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-apikey-and-ci

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.

@coveralls

coveralls commented Jul 1, 2026

Copy link
Copy Markdown

Coverage Status

Coverage is 99.624%fix-apikey-and-ci into master. No base build found for master.

coderabbitai[bot]
coderabbitai Bot previously requested changes Jul 1, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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/openwisp2/sample_users/migrations/0003_user_expiration_date_user_user_active_expiry_idx.py`:
- Around line 24-35: Add the missing rest_framework.authtoken migration
dependency for the ApiKey proxy model. Update the dependencies list in the
migration that defines CreateModel(name="ApiKey") so it also depends on
rest_framework.authtoken 0001_initial, ensuring authtoken.Token exists before
this proxy model is applied.
🪄 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

Run ID: 5a3983ec-540b-445a-9f51-4ece80ebe830

📥 Commits

Reviewing files that changed from the base of the PR and between 917cd4c and 522dd6b.

📒 Files selected for processing (3)
  • tests/openwisp2/sample_users/migrations/0002_default_groups_and_permissions.py
  • tests/openwisp2/sample_users/migrations/0003_user_expiration_date_user_user_active_expiry_idx.py
  • tests/openwisp2/sample_users/models.py
📜 Review details
⏰ Context from checks skipped due to timeout. (13)
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: Analyze (python)
  • GitHub Check: Python==3.12 | django~=4.2.0
  • GitHub Check: Python==3.12 | django~=5.1.0
  • GitHub Check: Python==3.11 | 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.10 | django~=5.2.0
  • GitHub Check: Python==3.10 | django~=5.1.0
  • GitHub Check: Python==3.13 | django~=5.1.0
  • GitHub Check: Python==3.13 | django~=5.2.0
  • GitHub Check: Python==3.10 | django~=4.2.0
  • GitHub Check: Python==3.11 | django~=5.1.0
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{py,html}

📄 CodeRabbit inference engine (Custom checks)

For Django pull requests, ensure all user-facing strings are marked as translatable using the Django i18n framework

Files:

  • tests/openwisp2/sample_users/models.py
  • tests/openwisp2/sample_users/migrations/0003_user_expiration_date_user_user_active_expiry_idx.py
  • tests/openwisp2/sample_users/migrations/0002_default_groups_and_permissions.py
**/*.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.
Mark user-facing strings for translation with Django i18n helpers in Django code.
Write comments and docstrings only when they explain why code is shaped a certain way. Put comments before the relevant code block instead of scattering them inside it.

Files:

  • tests/openwisp2/sample_users/models.py
  • tests/openwisp2/sample_users/migrations/0003_user_expiration_date_user_user_active_expiry_idx.py
  • tests/openwisp2/sample_users/migrations/0002_default_groups_and_permissions.py
🧠 Learnings (1)
📚 Learning: 2026-06-01T21:59:25.519Z
Learnt from: nemesifier
Repo: openwisp/openwisp-ipam PR: 218
File: tests/openwisp2/sample_users/migrations/0003_user_expiration_date_user_user_active_expiry_idx.py:17-21
Timestamp: 2026-06-01T21:59:25.519Z
Learning: In openwisp-ipam test project migration files under tests/openwisp2/sample_users/migrations/, do not wrap model field `verbose_name` and `help_text` strings with Django’s i18n helpers (e.g., `gettext_lazy`). Keep these strings as plain literals in these migrations; reviewers should not suggest adding translation wrappers for these test migration strings.

Applied to files:

  • tests/openwisp2/sample_users/migrations/0003_user_expiration_date_user_user_active_expiry_idx.py
  • tests/openwisp2/sample_users/migrations/0002_default_groups_and_permissions.py
🔇 Additional comments (2)
tests/openwisp2/sample_users/migrations/0002_default_groups_and_permissions.py (1)

6-6: LGTM!

Also applies to: 34-36

tests/openwisp2/sample_users/models.py (1)

4-6: LGTM!

@nemesifier nemesifier changed the title [tests:fix] Added ApiKey proxy model and permissions to sample users [fix] Add ApiKey proxy model and permissions to sample users Jul 1, 2026
@nemesifier

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@nemesifier
nemesifier dismissed coderabbitai[bot]’s stale review July 1, 2026 19:22

Addressed in 9470119. CodeRabbit marked the inline comment addressed and its status check is passing.

@nemesifier nemesifier self-assigned this Jul 1, 2026
@nemesifier nemesifier added the bug Something isn't working label Jul 1, 2026
@nemesifier nemesifier changed the title [fix] Add ApiKey proxy model and permissions to sample users [chores:fix] Add ApiKey proxy model and permissions to sample users Jul 1, 2026
@nemesifier

Copy link
Copy Markdown
Member Author

@coderabbitai resolve

coderabbitai[bot]
coderabbitai Bot previously approved these changes Jul 1, 2026
@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown
✅ Action performed

Comments resolved and changes approved.

@nemesifier
nemesifier force-pushed the fix-apikey-and-ci branch from 9470119 to f1032de Compare July 1, 2026 22:38
@nemesifier
nemesifier merged commit bf37e4c into master Jul 1, 2026
19 checks passed
@nemesifier
nemesifier deleted the fix-apikey-and-ci branch July 1, 2026 22:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants