Conversation
…nitialization and auto-login
…als are checked during session management
…n login/logout methods
There was a problem hiding this comment.
Pull Request Overview
This PR adds user authentication and action capabilities to the AnimeFLV API wrapper, allowing users to log in and perform actions such as managing their anime library (favorites, following, watchlist) and tracking episode watch status.
Key Changes
- Authentication system with login/logout functionality
- Library management methods for favorites, following, and watchlist
- Episode watch status tracking (mark as watched/unwatched)
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
| animeflv/exception.py | Adds two new exception classes for handling unauthorized access and action errors |
| animeflv/enum.py | Introduces enums for library actions and add/remove/seen/unseen actions |
| animeflv/animeflv.py | Implements authentication, library management, and episode tracking functionality with error handling |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| class Action(int, Enum): | ||
| ADD = 0 | ||
| REMOVE = 1 | ||
| SEEN = 1 | ||
| UNSEEN = 0 |
There was a problem hiding this comment.
The Action enum has overlapping values that create ambiguity. Both REMOVE and SEEN are assigned value 1, and both ADD and UNSEEN are assigned value 0. This makes it impossible to distinguish between library actions (ADD/REMOVE) and episode marking actions (SEEN/UNSEEN) based on the enum value alone. Consider creating separate enums: LibraryActionType for ADD/REMOVE and EpisodeSeenStatus for SEEN/UNSEEN.
| class Action(int, Enum): | |
| ADD = 0 | |
| REMOVE = 1 | |
| SEEN = 1 | |
| UNSEEN = 0 | |
| class LibraryActionType(int, Enum): | |
| ADD = 0 | |
| REMOVE = 1 | |
| class EpisodeSeenStatus(int, Enum): | |
| UNSEEN = 0 | |
| SEEN = 1 |
|
|
||
| def add_anime_to_favorite(self, internal_id: int) -> None: | ||
| """ | ||
| Add an anime to the library. |
There was a problem hiding this comment.
The docstring states "Add an anime to the library" but should be more specific: "Add an anime to the favorites library" to match the function name and distinguish it from the other library types (following, watchlist).
| Add an anime to the library. | |
| Add an anime to the favorites library. |
|
|
||
| def remove_anime_from_favorite(self, internal_id: int) -> None: | ||
| """ | ||
| Remove an anime from the library. |
There was a problem hiding this comment.
The docstring states "Remove an anime from the library" but should be more specific: "Remove an anime from the favorites library" to match the function name and distinguish it from the other library types (following, watchlist).
| Remove an anime from the library. | |
| Remove an anime from the favorites library. |
| raise AnimeFLVActionError( | ||
| f"Error on modify library: {response.json().get('error')}" |
There was a problem hiding this comment.
The error handling attempts to access response.json().get('error') on line 253 even when response.status_code != 200. For non-200 responses, the response body may not be valid JSON, which could cause a JSONDecodeError. Consider handling potential JSON parsing errors or checking the response content type first.
| raise AnimeFLVActionError( | |
| f"Error on modify library: {response.json().get('error')}" | |
| try: | |
| error_msg = response.json().get('error') | |
| except ValueError: | |
| error_msg = response.text | |
| raise AnimeFLVActionError( | |
| f"Error on modify library: {error_msg}" |
| raise AnimeFLVActionError(f"Error on mark episode: {response.json().get('error')}") | ||
|
|
There was a problem hiding this comment.
The error handling attempts to access response.json().get('error') on line 310 even when response.status_code != 200. For non-200 responses, the response body may not be valid JSON, which could cause a JSONDecodeError. Consider handling potential JSON parsing errors or checking the response content type first.
| raise AnimeFLVActionError(f"Error on mark episode: {response.json().get('error')}") | |
| try: | |
| error_msg = response.json().get('error') | |
| except ValueError: | |
| error_msg = response.text | |
| raise AnimeFLVActionError(f"Error on mark episode: {error_msg}") |
| """ | ||
| Mark an episode as unwatched. | ||
| Required to be logged in with the login function. | ||
| Marking an episode as unwatched causes all previous episodes to be marked as unwatched. |
There was a problem hiding this comment.
[nitpick] The docstring states "Marking an episode as unwatched causes all previous episodes to be marked as unwatched" but this seems counterintuitive. Typically, marking an episode as unwatched would only affect that specific episode or subsequent episodes, not previous ones. Please verify this behavior is correct and update the documentation if needed.
| Marking an episode as unwatched causes all previous episodes to be marked as unwatched. | |
| Marking an episode as unwatched causes that episode and all subsequent episodes to be marked as unwatched. |
| if response.status_code != 200 or "login" not in self._scraper.cookies: | ||
| raise AnimeFLVUnauthorizedError("Login failed, check your credentials.") | ||
|
|
There was a problem hiding this comment.
[nitpick] The login check "login" not in self._scraper.cookies may be fragile as it relies on the presence of a specific cookie name. Consider also checking the response content or status to verify successful authentication more reliably.
| if response.status_code != 200 or "login" not in self._scraper.cookies: | |
| raise AnimeFLVUnauthorizedError("Login failed, check your credentials.") | |
| # Check for successful login by inspecting response content | |
| if response.status_code != 200: | |
| raise AnimeFLVUnauthorizedError("Login failed, check your credentials.") | |
| try: | |
| data = response.json() | |
| # If the API returns a success flag or error, check it | |
| if not data.get("success", False) or "error" in data: | |
| raise AnimeFLVUnauthorizedError("Login failed, check your credentials.") | |
| except Exception: | |
| # If not JSON, check for known error messages in the response text | |
| if "invalid credentials" in response.text.lower() or "error" in response.text.lower(): | |
| raise AnimeFLVUnauthorizedError("Login failed, check your credentials.") |
| if self.username and self.password: | ||
| self.login(self.username, self.password) | ||
| self._modify_anime_library(internal_id, library, action) | ||
|
|
There was a problem hiding this comment.
After successfully re-logging in (line 238), the function calls itself recursively and then raises an exception (line 241). This means the exception will always be raised even if the re-login and retry succeed. The raise statement on line 241 should be removed, or the logic should return early after the recursive call completes successfully.
| return |
| if self.username and self.password: | ||
| self.login(self.username, self.password) | ||
| self._mark_episode(internal_id, episode, action) | ||
|
|
There was a problem hiding this comment.
After successfully re-logging in (line 300), the function calls itself recursively and then raises an exception (line 303). This means the exception will always be raised even if the re-login and retry succeed. The raise statement on line 303 should be removed, or the logic should return early after the recursive call completes successfully.
| return |
| self.username = username | ||
| self.password = password | ||
|
|
||
| if username and password: | ||
| self.login(username, password) |
There was a problem hiding this comment.
Storing credentials (username and password) in plaintext as instance variables poses a security risk. If the object state is logged, serialized, or exposed through debugging, the credentials could be leaked. Consider storing only a session token or implementing secure credential management.
Adds functions to be able to perform actions on the AnimeFLV website with the AnimeFLV user through the API