diff --git a/README.md b/README.md index 19f0c92..391f368 100644 --- a/README.md +++ b/README.md @@ -53,6 +53,11 @@ This update focuses on personalization and user experience. ### ⚡ Also in this update - Minor UI improvements and bug fixes for a more stable experience. +- Improved session stability after idle time: document lists and search results no longer disappear if a reload or search request fails. +- Main window startup loading was moved off the UI thread to reduce freezes during the initial data fetch. +- Authentication flows now fail safely if the system keyring or session storage is unavailable. +- API client now correctly handles empty successful responses such as `204 No Content`. +- Logging out now explicitly disables auto-login for the current profile. --- diff --git a/README_RU.md b/README_RU.md index b5fd14a..c2f1f19 100644 --- a/README_RU.md +++ b/README_RU.md @@ -53,6 +53,11 @@ ### ⚡ Также в этом обновлении - Улучшения интерфейса и исправления ошибок для более стабильной работы. +- Улучшена стабильность сессии после простоя: список документов и результаты поиска больше не исчезают, если запрос обновления или поиска завершился ошибкой. +- Начальная загрузка главного окна перенесена из UI-потока в фоновый, чтобы уменьшить подвисания при первом получении данных. +- Сценарии авторизации теперь корректно завершаются ошибкой, если системное хранилище ключей или сессии недоступно. +- API-клиент теперь корректно обрабатывает успешные пустые ответы, например `204 No Content`. +- При выходе из аккаунта теперь явно отключается auto-login для текущего профиля. --- diff --git a/api/api_client.py b/api/api_client.py index 84e6ce8..fb446ab 100644 --- a/api/api_client.py +++ b/api/api_client.py @@ -537,4 +537,6 @@ def _request(self, method: str, url: str, timeout: int = 10, **kwargs) -> dict: """Generic method for making API requests.""" request = self.session.request(method, url, timeout=timeout, **kwargs) request.raise_for_status() - return request.json() \ No newline at end of file + if request.status_code == 204 or not request.content: + return {} + return request.json() diff --git a/modules/auth/mvc/auth_controller.py b/modules/auth/mvc/auth_controller.py index 3ae87d0..5a19bdd 100644 --- a/modules/auth/mvc/auth_controller.py +++ b/modules/auth/mvc/auth_controller.py @@ -93,15 +93,15 @@ def login_user(self, data: dict) -> None: Args: data (dict): The user data received from the API upon login. """ - # Save user data - auto_login = self.view.login_page.get_auto_login_state() - user_id = self.model.save_user(user_data=data, auto_login=auto_login) + try: + auto_login = self.view.login_page.get_auto_login_state() + user_id = self.model.save_user(user_data=data, auto_login=auto_login) - # Clear lineedits - self.view.login_page.clear_lineedits() - - # Switch to main window - self.login_successful.emit("auth", user_id) + self.view.login_page.clear_lineedits() + self.login_successful.emit("auth", user_id) + except Exception as e: + msg = get_friendly_error_message(e) + NotificationService().show_toast("error", "Ошибка входа", msg) @@ -117,20 +117,21 @@ def signup_user(self, user_data: dict) -> None: Args: user_data (dict): The user data received from the API upon signup. """ - # Save user data - auto_login = self.view.signup_page.get_auto_login_state() - user_id = self.model.save_user(user_data=user_data, auto_login=auto_login) - - # Clear lineedits - self.view.signup_page.clear_lineedits() - - NotificationService().show_toast( - notification_type="success", - title="Регистрация успешна", - message="Аккаунт создан. Добро пожаловать!" - ) - # Switch to main window - self.login_successful.emit("auth", user_id) + try: + auto_login = self.view.signup_page.get_auto_login_state() + user_id = self.model.save_user(user_data=user_data, auto_login=auto_login) + + self.view.signup_page.clear_lineedits() + + NotificationService().show_toast( + notification_type="success", + title="Регистрация успешна", + message="Аккаунт создан. Добро пожаловать!" + ) + self.login_successful.emit("auth", user_id) + except Exception as e: + msg = get_friendly_error_message(e) + NotificationService().show_toast("error", "Ошибка регистрации", msg) def signup(self, data: dict, email: str, password: str) -> None: @@ -203,13 +204,15 @@ def switch_to_reset_password_page(self, data: dict) -> None: Args: data (dict): Data containing the reset token from the API. """ - # Save token - self.model.save_token(token_name="reset_token", token="reset_token", data=data) + try: + self.model.save_token(token_name="reset_token", token="reset_token", data=data) - # Switch to change password page - page = self.view.pages.get("change_password_change_page", None) - if page: - self.view.switch_page(page=page) + page = self.view.pages.get("change_password_change_page", None) + if page: + self.view.switch_page(page=page) + except Exception as e: + msg = get_friendly_error_message(e) + NotificationService().show_toast("error", "Ошибка восстановления", msg) def open_email_confirm_modal_window(self, data, email: str) -> None: @@ -653,4 +656,4 @@ def _worker_error(self, exception: Exception) -> None: notification_type="error", title="Ошибка", message=message - ) \ No newline at end of file + ) diff --git a/modules/auth/mvc/auth_model.py b/modules/auth/mvc/auth_model.py index 5952041..ecf4bd5 100644 --- a/modules/auth/mvc/auth_model.py +++ b/modules/auth/mvc/auth_model.py @@ -259,6 +259,16 @@ def delete_file(file_path: Path) -> None: if file_path.exists(): file_path.unlink() + def disable_auto_login(user_id: int) -> None: + profile_path = self.APP_DIR / "Profiles" / f"user_data_{user_id}.json" + profile_data = read_json(profile_path) + if not isinstance(profile_data, dict): + return + + profile_data["auto_login"] = False + with open(profile_path, "w", encoding="utf-8") as f: + json.dump(profile_data, f, indent=4, ensure_ascii=False) + # Get last user id last_logged_data = read_json(self.LOCAL_DIR_LAST_LOGGED) if not last_logged_data: @@ -276,6 +286,8 @@ def delete_file(file_path: Path) -> None: except keyring_errors.PasswordDeleteError: logging.info(f"Tokens for user_id {user_id} not found in keyring, skipping deletion.") + disable_auto_login(user_id) + # Delete the last logged user file to disable auto-login on next start delete_file(self.LOCAL_DIR_LAST_LOGGED) logging.info("Last logged user file deleted to disable auto-login. User is fully logged out.") @@ -376,7 +388,9 @@ def save_token(self, token_name: str, token: str, data: dict) -> None: password=data.get(token, None) ) - except keyring_errors.PasswordSetError: + except keyring_errors.PasswordSetError as e: logging.error(msg="PasswordSetError", exc_info=True) + raise RuntimeError("Не удалось сохранить данные сессии в системное хранилище.") from e except Exception as e: - logging.error(msg=e, exc_info=True) \ No newline at end of file + logging.error(msg=e, exc_info=True) + raise RuntimeError("Не удалось сохранить данные сессии в системное хранилище.") from e diff --git a/modules/main/mvc/main_controller.py b/modules/main/mvc/main_controller.py index 40bd78b..c00ccde 100644 --- a/modules/main/mvc/main_controller.py +++ b/modules/main/mvc/main_controller.py @@ -67,6 +67,7 @@ def __init__( self.current_documents = [] self.current_search_worker = None self.current_load_worker = None + self.initial_load_worker = None self.active_workers = set() self.is_updating_data = False @@ -788,6 +789,7 @@ def _on_search_error(self, error: Exception) -> None: """Handles search errors.""" if self.sender() != self.current_search_worker: return + self.current_search_worker = None self._handle_error(error, "Ошибка поиска") def _cleanup_worker(self) -> None: @@ -804,7 +806,6 @@ def _cleanup_worker(self) -> None: def _init_ui(self) -> None: """Initializes the UI with default data.""" - self._load_sidebar_data() self.view.set_profile_mode(self.mode) # Load and apply settings @@ -812,45 +813,79 @@ def _init_ui(self) -> None: search_filters = self.settings_manager.get_setting("search_filters") if search_filters: self.view.set_search_filters(search_filters) + self._load_initial_data() - # Set user data - if self.mode == "auth": - user_data = self.model.get_user_data() - - if user_data: - username = user_data.get("username") - department_name = user_data.get("department") - self.view.set_username(name=username if username else user_data.get("email")) - self.view.set_user_department(dept=department_name if department_name else "Отдел не выбран") - - # Find the user's department ID from the name - user_dept_id = None - if department_name: - for dept in self.model.departments: - if dept.get("name") == department_name: - user_dept_id = dept.get("id") - break - - # If a department is found, set it as current by triggering selection - if user_dept_id: - # Use a timer to ensure the UI is ready for selection. - # This will trigger the _on_department_selected slot, - # which will then update the model and load the data. - QTimer.singleShot(50, lambda: self.view.select_department(user_dept_id)) - # If user has no department, select the first one in the list - elif self.model.departments: - first_dept_id = self.model.departments[0].get("id") - if first_dept_id: - QTimer.singleShot(50, lambda: self.view.select_department(first_dept_id)) + def _load_initial_data(self) -> None: + """Loads the initial sidebar and profile data without blocking the UI thread.""" + worker = APIWorker(self.model.load_initial_data) + self.initial_load_worker = worker + self.active_workers.add(worker) + worker.finished.connect(self._on_initial_data_loaded) + worker.error.connect(self._on_initial_data_error) + worker.finished.connect(self._cleanup_worker) + worker.error.connect(self._cleanup_worker) + worker.start() + def _on_initial_data_loaded(self, data: dict) -> None: + if self.sender() != self.initial_load_worker: + return + + self.initial_load_worker = None + self.model.departments = data.get("departments", []) + self.model.categories = data.get("categories", []) + self.model.current_department_id = self.model.departments[0]["id"] if self.model.departments else None + self._load_sidebar_data() + + if self.mode == "auth": + self._apply_user_data(data.get("user_data")) else: self.view.set_username("Гость") self.view.set_user_department("Войдите в аккаунт") + if self.model.departments: + first_dept_id = self.model.departments[0].get("id") + if first_dept_id: + QTimer.singleShot(50, lambda: self.view.select_department(first_dept_id)) - # Set documents data self._update_create_category_state() self._update_create_document_state() + def _on_initial_data_error(self, error: Exception) -> None: + if self.sender() != self.initial_load_worker: + return + + self.initial_load_worker = None + if self.mode == "auth": + self.view.set_username("Пользователь") + self.view.set_user_department("Не удалось загрузить данные") + else: + self.view.set_username("Гость") + self.view.set_user_department("Войдите в аккаунт") + self._handle_error(error, "Ошибка загрузки данных") + + def _apply_user_data(self, user_data: dict | None) -> None: + """Applies the loaded user data to the UI and restores sidebar selection.""" + if not user_data: + return + + username = user_data.get("username") + department_name = user_data.get("department") + self.view.set_username(name=username if username else user_data.get("email")) + self.view.set_user_department(dept=department_name if department_name else "Отдел не выбран") + + user_dept_id = None + if department_name: + for dept in self.model.departments: + if dept.get("name") == department_name: + user_dept_id = dept.get("id") + break + + if user_dept_id: + QTimer.singleShot(50, lambda: self.view.select_department(user_dept_id)) + elif self.model.departments: + first_dept_id = self.model.departments[0].get("id") + if first_dept_id: + QTimer.singleShot(50, lambda: self.view.select_department(first_dept_id)) + def _setup_connections(self) -> None: """Sets up signal-slot connections.""" @@ -954,20 +989,26 @@ def _update_categories_list(self) -> None: def _update_documents_list(self) -> None: """Updates the documents list based on the current category.""" self.model.current_document_id = None - - # Reset pagination + + if self.model.current_category_id is None: + self.offset = 0 + self.has_more = False + self.current_documents = [] + self.search_results_all = [] + self.pending_documents_to_add = [] + self.view.clear_documents_table() + self.view.set_finded_counter(0) + self.view.set_active_search_tags([]) + self.view.set_export_print_enabled(False) + return + self.offset = 0 self.has_more = True - self.current_documents = [] - self.view.clear_documents_table() - self.view.set_finded_counter(0) - self.view.set_active_search_tags([]) - self.view.set_export_print_enabled(False) - - # Reset loading state to ensure we can load new data + self.is_loading = False self.current_load_worker = None - + self.pending_documents_to_add = [] + self._load_more_documents() @@ -1013,6 +1054,7 @@ def _on_load_more_finished(self, docs: list) -> None: self.is_loading = False self.current_load_worker = None + self.search_results_all = [] # Prevent updating UI if search is active (stale request) if self.view.get_search_text(): @@ -1029,6 +1071,7 @@ def _on_load_more_finished(self, docs: list) -> None: # Instead of one large, blocking update, we add the data in chunks # to keep the UI responsive. self.view.clear_documents_table() + self.view.set_active_search_tags([]) self.pending_documents_to_add = list(self.current_documents) # Start the chunked update. Use a single shot timer to yield to the event loop. QTimer.singleShot(0, self._add_document_chunk) @@ -1059,6 +1102,7 @@ def _on_load_more_error(self, error: Exception) -> None: return self.is_loading = False + self.current_load_worker = None self._handle_error(error, "Ошибка загрузки документов") @@ -1194,4 +1238,4 @@ def _handle_error(self, e: Exception, title: str = "Ошибка") -> None: notification_type="error", title=title, message=msg - ) \ No newline at end of file + ) diff --git a/modules/main/mvc/main_model.py b/modules/main/mvc/main_model.py index d9087c5..0729f99 100644 --- a/modules/main/mvc/main_model.py +++ b/modules/main/mvc/main_model.py @@ -24,10 +24,10 @@ def __init__(self, mode: str = "guest") -> None: self.LOCAL_DIR_LAST_LOGGED = self.LOCAL_DIR / "last_logged.json" # Sidebar data - self.departments = self._get_departments() - self.current_department_id = self.departments[0]["id"] if self.departments else None - self.categories = self._get_categories() - + self.departments = [] + self.current_department_id = None + self.categories = [] + self.current_category_id = None # Table data @@ -46,6 +46,17 @@ def get_user_data(self) -> dict | None: data = self.api.get_user_data(token) return data + + def load_initial_data(self) -> dict: + """Loads the sidebar data and current user profile for the startup flow.""" + departments = self._get_departments() + categories = self._get_categories() + user_data = self.get_user_data() if self.mode == "auth" else None + return { + "departments": departments, + "categories": categories, + "user_data": user_data, + } def get_document(self, document_id: int) -> dict: @@ -161,6 +172,7 @@ def delete_category(self, category_id: int): def refresh_data(self) -> None: """Refreshes the data from the API.""" self.departments = self._get_departments() + self.current_department_id = self.departments[0]["id"] if self.departments else None self.categories = self._get_categories() @@ -328,4 +340,4 @@ def _get_departments(self) -> list[dict]: def _get_categories(self) -> list[dict]: categories = self._make_authorized_request(self.api.get_categories) return categories["categories"] - \ No newline at end of file + diff --git a/tests/test_apiclient.py b/tests/test_apiclient.py index 01edb08..5c078a7 100644 --- a/tests/test_apiclient.py +++ b/tests/test_apiclient.py @@ -146,6 +146,30 @@ def test_delete_document(self, client): headers={"Authorization": f"Bearer {token}"} ) + def test_request_returns_empty_dict_for_204(self, client): + with patch.object(client.session, "request") as mock_request: + mock_response = Mock() + mock_response.status_code = 204 + mock_response.content = b"" + mock_response.raise_for_status = Mock() + mock_request.return_value = mock_response + + result = client.delete_document(123, "auth_token") + + assert result == {} + + def test_request_returns_empty_dict_for_empty_body(self, client): + with patch.object(client.session, "request") as mock_request: + mock_response = Mock() + mock_response.status_code = 200 + mock_response.content = b"" + mock_response.raise_for_status = Mock() + mock_request.return_value = mock_response + + result = client._request("DELETE", f"{self.BASE_URL}/app/files/1") + + assert result == {} + def test_search_data(self, client): token = "auth_token" query = "test query" @@ -434,4 +458,4 @@ def test_update_document(self, client): assert result == expected mock_request.assert_called_once_with( "PATCH", f"{self.BASE_URL}/app/documents/{doc_id}", timeout=10, headers={"Authorization": f"Bearer {token}"}, json=data - ) \ No newline at end of file + ) diff --git a/tests/test_auth.py b/tests/test_auth.py index d2d3a94..c77ca76 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -59,18 +59,27 @@ def test_save_user(self, model): # Check file writing (user data and last logged) assert mock_open.call_count >= 2 + def test_save_token_raises_when_keyring_write_fails(self, model): + with patch("modules.auth.mvc.auth_model.keyring.set_password", side_effect=Exception("keyring fail")): + with pytest.raises(RuntimeError): + model.save_token("access_token_1", "access_token", {"access_token": "acc"}) + def test_logout(self, model): """Test logout clears keyring and deletes last logged file.""" # Configure the mock path object to simulate file existence model.LOCAL_DIR_LAST_LOGGED.exists.return_value = True - with patch("modules.auth.mvc.auth_model.read_json", return_value={"user_id": 1}), \ + with patch("modules.auth.mvc.auth_model.read_json", side_effect=[{"user_id": 1}, {"auto_login": True}]), \ patch("modules.auth.mvc.auth_model.keyring.delete_password") as mock_keyring_del: - model.logout() + with patch("builtins.open", new_callable=MagicMock) as mock_open, \ + patch("json.dump") as mock_json_dump: + model.logout() assert mock_keyring_del.call_count == 2 + assert mock_open.call_count >= 1 + mock_json_dump.assert_called() model.LOCAL_DIR_LAST_LOGGED.unlink.assert_called_once() @@ -145,6 +154,31 @@ def test_login_success_callback(self, controller): controller.view.login_page.clear_lineedits.assert_called_once() mock_signal.assert_called_once_with("auth", 1) + def test_login_success_callback_handles_save_error(self, controller): + data = {"user": {"id": 1}} + controller.view.login_page.get_auto_login_state.return_value = True + controller.model.save_user.side_effect = RuntimeError("keyring fail") + + mock_signal = Mock() + controller.login_successful.connect(mock_signal) + + with patch("modules.auth.mvc.auth_controller.NotificationService") as MockNotify: + controller.login_user(data) + + mock_signal.assert_not_called() + controller.view.login_page.clear_lineedits.assert_not_called() + MockNotify.return_value.show_toast.assert_called_once() + + def test_reset_password_page_switch_handles_save_error(self, controller): + controller.view.pages = {"change_password_change_page": Mock()} + controller.model.save_token.side_effect = RuntimeError("keyring fail") + + with patch("modules.auth.mvc.auth_controller.NotificationService") as MockNotify: + controller.switch_to_reset_password_page({"reset_token": "token"}) + + controller.view.switch_page.assert_not_called() + MockNotify.return_value.show_toast.assert_called_once() + def test_signup_flow(self, controller): """Test signup button click initiates code sending.""" @@ -181,4 +215,4 @@ def test_validation_login_invalid(self, controller): controller.view.login_page.get_email.return_value = "invalid" controller.on_login_page_lineedits_changed() - controller.view.login_page.update_submit_button_state.assert_not_called() \ No newline at end of file + controller.view.login_page.update_submit_button_state.assert_not_called() diff --git a/tests/test_main.py b/tests/test_main.py index 570b7a8..76bf886 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -20,21 +20,45 @@ def controller(self, qapp): # Mock NotificationService and QTimer to avoid side effects with patch("modules.main.mvc.main_controller.NotificationService"), \ - patch("modules.main.mvc.main_controller.QTimer"): + patch("modules.main.mvc.main_controller.QTimer"), \ + patch("modules.main.mvc.main_controller.APIWorker"): ctrl = MainController(model, view, window, mode="auth") yield ctrl def test_init_ui(self, controller): - """Test UI initialization loads sidebar and user data.""" - controller.model.get_user_data.return_value = {"username": "User", "department": "IT"} - - controller._init_ui() - + """Test UI initialization starts background loading instead of blocking.""" + with patch("modules.main.mvc.main_controller.APIWorker") as MockWorker: + controller._init_ui() + + MockWorker.assert_called_once_with(controller.model.load_initial_data) + MockWorker.return_value.start.assert_called_once() + + def test_initial_data_loaded(self, controller): + """Test applying startup data after the background worker finishes.""" + controller.initial_load_worker = Mock() + with patch.object(controller, "sender", return_value=controller.initial_load_worker): + controller._on_initial_data_loaded({ + "departments": [{"id": 1, "name": "Dept 1", "documents_count": 5}], + "categories": [{"id": 10, "name": "Cat 1", "group_id": 1, "documents_count": 2}], + "user_data": {"username": "User", "department": "Dept 1"}, + }) + controller.view.update_departments.assert_called() controller.view.update_categories.assert_called() controller.view.set_username.assert_called_with(name="User") + def test_initial_data_error(self, controller): + """Test startup error handling leaves the window in a safe state.""" + controller.initial_load_worker = Mock() + with patch.object(controller, "sender", return_value=controller.initial_load_worker), \ + patch.object(controller, "_handle_error") as mock_handle_error: + controller._on_initial_data_error(Exception("boom")) + + controller.view.set_username.assert_called_with("Пользователь") + controller.view.set_user_department.assert_called_with("Не удалось загрузить данные") + mock_handle_error.assert_called_once() + def test_search_trigger(self, controller): """Test that typing in search triggers the search worker.""" @@ -62,7 +86,6 @@ def test_department_selection(self, controller): assert controller.model.current_department_id == 2 assert controller.model.current_category_id is None - controller.view.clear_documents_table.assert_called() controller.view.update_categories.assert_called() @@ -106,6 +129,48 @@ def test_load_more_documents(self, controller): assert kwargs['offset'] == 0 assert kwargs['limit'] == controller.limit + def test_update_documents_list_keeps_previous_table_until_success(self, controller): + """Reloading the table should not clear the old data before the new response arrives.""" + controller.model.current_category_id = 10 + controller.current_documents = [{"id": 1, "name": "Doc"}] + controller.view.get_search_text.return_value = "" + controller.view.clear_documents_table.reset_mock() + + with patch.object(controller, "_load_more_documents") as mock_load_more: + controller._update_documents_list() + + assert controller.current_documents == [{"id": 1, "name": "Doc"}] + controller.view.clear_documents_table.assert_not_called() + mock_load_more.assert_called_once() + + def test_load_more_error_keeps_existing_documents_visible(self, controller): + """A failed reload should not wipe the currently visible documents.""" + controller.current_documents = [{"id": 1, "name": "Existing"}] + controller.current_load_worker = Mock() + controller.view.clear_documents_table.reset_mock() + + with patch.object(controller, "sender", return_value=controller.current_load_worker), \ + patch.object(controller, "_handle_error") as mock_handle_error: + controller._on_load_more_error(Exception("boom")) + + assert controller.current_documents == [{"id": 1, "name": "Existing"}] + controller.view.clear_documents_table.assert_not_called() + mock_handle_error.assert_called_once() + + def test_search_error_keeps_existing_documents_visible(self, controller): + """A failed search should leave the previously rendered table intact.""" + controller.current_documents = [{"id": 1, "name": "Existing"}] + controller.current_search_worker = Mock() + controller.view.clear_documents_table.reset_mock() + + with patch.object(controller, "sender", return_value=controller.current_search_worker), \ + patch.object(controller, "_handle_error") as mock_handle_error: + controller._on_search_error(Exception("boom")) + + assert controller.current_documents == [{"id": 1, "name": "Existing"}] + controller.view.clear_documents_table.assert_not_called() + mock_handle_error.assert_called_once() + def test_create_department_flow(self, controller): """Test create department dialog and model update.""" @@ -178,4 +243,4 @@ def test_logout(self, controller): mock_slot = Mock() controller.logout_requested.connect(mock_slot) controller._on_logout_clicked() - mock_slot.assert_called_once() \ No newline at end of file + mock_slot.assert_called_once()