Skip to content

Comments

test: add tests for latest features#58

Merged
PascalRepond merged 1 commit intomainfrom
rep-dev
Jan 21, 2026
Merged

test: add tests for latest features#58
PascalRepond merged 1 commit intomainfrom
rep-dev

Conversation

@PascalRepond
Copy link
Owner

No description provided.

@coderabbitai
Copy link

coderabbitai bot commented Jan 21, 2026

📝 Walkthrough

Walkthrough

Adds new tests for the saved_views context processor and MusicBrainz search view, and replaces inline back-button markup with a shared partial across several templates (plus a new back_button partial). Also disables autocomplete on certain media_import inputs and adjusts profile header markup.

Changes

Cohort / File(s) Summary
Core test modules
src/tests/core/test_context_processors.py, src/tests/core/test_musicbrainz.py
Adds TestSavedViewsContextProcessor (4 tests: authenticated/anonymous behavior, isolation, availability across pages) and TestMusicBrainzSearchView (6 tests: short/empty queries, mocked search results, API error handling, preserves media_id/query).
Templates — back button partial
src/templates/partials/common/back_button.html
New shared back-button partial added (history.back fallback to home, i18n, icon, aria-label).
Templates — replace inline back buttons
src/templates/accounts/profile_edit.html, src/templates/base/backup_manage.html, src/templates/base/media_detail.html, src/templates/base/media_edit.html, src/templates/base/media_import.html
Replaces inline back-button markup with included partial; moves back button into headers for profile_edit and backup_manage; removes duplicated bottom "Back to Home" link in profile edit.
Templates — media_import inputs
src/templates/base/media_import.html
Adds autocomplete="off" to primary source tab inputs and individual search inputs (tmdb/igdb/openlibrary/musicbrainz).

Sequence Diagram(s)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Description check ⚠️ Warning No pull request description was provided by the author, making it impossible to assess whether the description relates to the changeset. Add a pull request description explaining the purpose of the tests, which features are being tested, and the scope of the changes.
Title check ❓ Inconclusive The title 'test: add tests for latest features' is vague and generic, using non-descriptive language that doesn't convey meaningful information about which specific features are being tested. Replace with a more specific title that identifies the key features being tested, such as 'test: add context processor and MusicBrainz integration tests' or similar.
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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 and usage tips.

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: 1

🤖 Fix all issues with AI agents
In `@src/tests/core/test_musicbrainz.py`:
- Around line 76-81: The test_preserves_query_in_context currently makes an
unmocked request and may hit the real MusicBrainz API; update the test to mock
the MusicBrainz client used by the view (same approach as
test_returns_search_results) so the view path that performs searches is stubbed
out — e.g., patch the client/class or method the view imports (referencing the
test function name test_preserves_query_in_context and the view
reverse("musicbrainz_search_htmx")) and ensure the mock returns an
empty/controlled response before calling logged_in_client.get(..., {"q":"test"})
so no network calls occur and the assertion on response.context["query"] remains
valid.
🧹 Nitpick comments (2)
src/tests/core/test_context_processors.py (1)

52-63: Consider moving the import to the top of the file.

The test logic is correct. However, the AnonymousUser import on line 54 could be moved to the module's import section for consistency.

♻️ Suggested refactor
 from django.test import RequestFactory
+from django.contrib.auth.models import AnonymousUser
 
 from core.context_processors import saved_views
 from core.models import SavedView

Then remove the local import on line 54.

src/tests/core/test_musicbrainz.py (1)

33-54: Consider verifying the search query argument.

The test correctly mocks the MusicBrainz client and verifies results. For more precise verification, you could use assert_called_once_with to confirm the correct query was passed to the client.

♻️ Suggested improvement
-        mock_client.search_releases.assert_called_once()
+        mock_client.search_releases.assert_called_once_with("abbey road")

Also harmonises back buttons across the application.
@PascalRepond PascalRepond merged commit b013b4f into main Jan 21, 2026
3 checks passed
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