From 1cae984f10fb788a20bed8b9d14b541a4b4500b0 Mon Sep 17 00:00:00 2001 From: David Foster Date: Sun, 21 Jul 2024 11:04:41 -0400 Subject: [PATCH 1/8] New Root URL: Refactor privatize fields --- src/crystal/browser/new_root_url.py | 75 ++++++++++++++--------------- 1 file changed, 37 insertions(+), 38 deletions(-) diff --git a/src/crystal/browser/new_root_url.py b/src/crystal/browser/new_root_url.py index 0d9ca57d..9e3f3080 100644 --- a/src/crystal/browser/new_root_url.py +++ b/src/crystal/browser/new_root_url.py @@ -42,11 +42,10 @@ class NewRootUrlDialog: # NOTE: Only changed when tests are running _last_opened: 'Optional[NewRootUrlDialog]'=None - # TODO: Privatize these fields - url_field: wx.TextCtrl - name_field: wx.TextCtrl - ok_button: wx.Button - cancel_button: wx.Button + _url_field: wx.TextCtrl + _name_field: wx.TextCtrl + _ok_button: wx.Button + _cancel_button: wx.Button _options_button: wx.Button _set_as_default_domain_checkbox: wx.CheckBox @@ -123,9 +122,9 @@ def __init__(self, if not fields_hide_hint_when_focused(): # Initialize focus if not is_edit: - self.url_field.SetFocus() + self._url_field.SetFocus() else: - self.name_field.SetFocus() + self._name_field.SetFocus() position_dialog_initially(dialog) # TODO: Verify that the wxGTK-specific logic here is actually necessary @@ -164,19 +163,19 @@ def _create_fields(self, parent: wx.Window, initial_url: str, initial_name: str, url_field_and_spinner = wx.BoxSizer(wx.HORIZONTAL) spinner_diameter: int if True: - self.url_field = wx.TextCtrl( + self._url_field = wx.TextCtrl( parent, value=initial_url, size=(self._INITIAL_URL_WIDTH, wx.DefaultCoord), name='cr-new-root-url-dialog__url-field') - self.url_field.Hint = 'https://example.com/' - self.url_field.SetSelection(-1, -1) # select all upon focus - self.url_field.Enabled = not is_edit - bind(self.url_field, wx.EVT_TEXT, self._update_ok_enabled) - bind(self.url_field, wx.EVT_SET_FOCUS, self._on_url_field_focus) - bind(self.url_field, wx.EVT_KILL_FOCUS, self._on_url_field_blur) - url_field_and_spinner.Add(self.url_field, proportion=1, flag=wx.EXPAND) + self._url_field.Hint = 'https://example.com/' + self._url_field.SetSelection(-1, -1) # select all upon focus + self._url_field.Enabled = not is_edit + bind(self._url_field, wx.EVT_TEXT, self._update_ok_enabled) + bind(self._url_field, wx.EVT_SET_FOCUS, self._on_url_field_focus) + bind(self._url_field, wx.EVT_KILL_FOCUS, self._on_url_field_blur) + url_field_and_spinner.Add(self._url_field, proportion=1, flag=wx.EXPAND) - spinner_diameter = self.url_field.Size.Height + spinner_diameter = self._url_field.Size.Height self.url_cleaner_spinner = wx.ActivityIndicator( parent, size=wx.Size(spinner_diameter, spinner_diameter), @@ -192,12 +191,12 @@ def _create_fields(self, parent: wx.Window, initial_url: str, initial_name: str, name_field_and_space = wx.BoxSizer(wx.HORIZONTAL) if True: - self.name_field = wx.TextCtrl( + self._name_field = wx.TextCtrl( parent, value=initial_name, name='cr-new-root-url-dialog__name-field') - self.name_field.Hint = 'Home' - self.name_field.SetSelection(-1, -1) # select all upon focus - name_field_and_space.Add(self.name_field, proportion=1, flag=wx.EXPAND) + self._name_field.Hint = 'Home' + self._name_field.SetSelection(-1, -1) # select all upon focus + name_field_and_space.Add(self._name_field, proportion=1, flag=wx.EXPAND) #name_field_and_space.Add( # wx.Size(spinner_diameter, spinner_diameter), @@ -261,8 +260,8 @@ def _create_buttons(self, parent: wx.Window, is_edit: bool) -> wx.Sizer: button_sizer.AddStretchSpacer() button_sizer.Add(CreateButtonSizer(parent, ok_button_id, wx.ID_CANCEL), flag=wx.CENTER) - self.ok_button = parent.FindWindow(id=ok_button_id) - self.cancel_button = parent.FindWindow(id=wx.ID_CANCEL) + self._ok_button = parent.FindWindow(id=ok_button_id) + self._cancel_button = parent.FindWindow(id=wx.ID_CANCEL) return button_sizer @@ -292,15 +291,15 @@ def _set_cleaned_url(self, cleaned_url: str) -> None: return self._last_cleaned_url = cleaned_url - if self.url_field.Value != cleaned_url: - self.url_field.Value = cleaned_url + if self._url_field.Value != cleaned_url: + self._url_field.Value = cleaned_url # === Events === # NOTE: Focus event can be called multiple times without an intermediate blur event @fg_affinity def _on_url_field_focus(self, event: wx.FocusEvent) -> None: - # NOTE: Cannot use url_field.HasFocus() because doesn't work in automated tests + # NOTE: Cannot use _url_field.HasFocus() because doesn't work in automated tests if self._url_field_focused: # Already did focus action return @@ -310,7 +309,7 @@ def _on_url_field_focus(self, event: wx.FocusEvent) -> None: # stop cleaning any old URL input @capture_crashes_to_stderr # no good location in UI to route crashes too def fg_task() -> None: - # NOTE: Cannot use url_field.HasFocus() because doesn't work in automated tests + # NOTE: Cannot use _url_field.HasFocus() because doesn't work in automated tests if not self._url_field_focused: return @@ -336,7 +335,7 @@ def _on_url_field_blur(self, event: Optional[wx.FocusEvent]=None) -> None: return # Start cleaning the new URL input - url_input = self.url_field.Value + url_input = self._url_field.Value if url_input == self._last_cleaned_url: # URL is already clean pass @@ -388,16 +387,16 @@ def _on_ok(self, event: Optional[wx.CommandEvent]=None) -> None: # If URL input is being cleaned, wait for it to finish before continuing if self._url_cleaner is not None: - self.url_field.Enabled = False - self.name_field.Enabled = False - self.ok_button.Enabled = False - assert self.cancel_button.Enabled == True + self._url_field.Enabled = False + self._name_field.Enabled = False + self._ok_button.Enabled = False + assert self._cancel_button.Enabled == True self._was_ok_pressed = True return - name = self.name_field.Value - url = self.url_field.Value + name = self._name_field.Value + url = self._url_field.Value if not self._is_edit and self._url_exists_func(url): dialog = wx.MessageDialog( self.dialog, @@ -410,10 +409,10 @@ def _on_ok(self, event: Optional[wx.CommandEvent]=None) -> None: choice = ShowModal(dialog) assert wx.ID_OK == choice - self.url_field.Enabled = True - self.name_field.Enabled = True - self.ok_button.Enabled = True - assert self.cancel_button.Enabled == True + self._url_field.Enabled = True + self._name_field.Enabled = True + self._ok_button.Enabled = True + assert self._cancel_button.Enabled == True return if self._set_as_default_domain_checkbox.IsChecked(): change_prefix_command = ('domain', url) # type: ChangePrefixCommand @@ -474,7 +473,7 @@ def _on_destroyed(self, event) -> None: # === Updates === def _update_ok_enabled(self, event=None) -> None: - self.ok_button.Enabled = (self.url_field.Value != '') + self._ok_button.Enabled = (self._url_field.Value != '') def fields_hide_hint_when_focused() -> bool: From ed6ad9d8a60e68c83898a55033d1db60f5c19db1 Mon Sep 17 00:00:00 2001 From: David Foster Date: Thu, 22 Aug 2024 22:22:18 -0400 Subject: [PATCH 2/8] Refactor extract MockHttpServer to test utilities --- src/crystal/tests/test_download.py | 74 +----------------------- src/crystal/tests/test_new_root_url.py | 2 +- src/crystal/tests/util/server.py | 78 +++++++++++++++++++++++++- 3 files changed, 77 insertions(+), 77 deletions(-) diff --git a/src/crystal/tests/test_download.py b/src/crystal/tests/test_download.py index 28771fac..58a870b6 100644 --- a/src/crystal/tests/test_download.py +++ b/src/crystal/tests/test_download.py @@ -6,18 +6,14 @@ from crystal.tests.util.asserts import assertEqual from crystal.tests.util.controls import click_button, TreeItem from crystal.tests.util.runner import bg_sleep -from crystal.tests.util.server import served_project +from crystal.tests.util.server import MockHttpServer, served_project from crystal.tests.util.tasks import wait_for_download_to_start_and_finish from crystal.tests.util.wait import DEFAULT_WAIT_PERIOD, wait_for from crystal.tests.util.windows import NewGroupDialog, OpenOrCreateDialog -from crystal.util.bulkheads import capture_crashes_to_stderr -from crystal.util.xthreading import bg_call_later -from http.server import BaseHTTPRequestHandler, HTTPServer import io import os import tempfile from textwrap import dedent -from typing import List from unittest import skip from unittest.mock import patch @@ -441,72 +437,4 @@ def finish(self, *args, **kwargs): assert 1 == DownloadResourceGroupTaskSpy.finish_call_count -# ------------------------------------------------------------------------------ -# Utility - -class MockHttpServer: - def __init__(self, routes) -> None: - self.requested_paths = [] # type: List[str] - - mock_server = self # capture - class RequestHandler(BaseHTTPRequestHandler): - def do_GET(self) -> None: # override - mock_server.requested_paths.append(self.path) - - # Look for static route - route = routes.get(self.path) - if route is not None: - self._send_route_response(route) - return - - # Look for dynamic route - for (path_pattern, route) in routes.items(): - if not callable(path_pattern): - continue - if not path_pattern(self.path): - continue - self._send_route_response(route) - return - - self.send_response(404) - self.end_headers() - return - - def _send_route_response(self, route) -> None: - self.send_response(route['status_code']) - for (k, v) in route['headers']: - self.send_header(k, v) - self.end_headers() - - content = route['content'] - if callable(content): - content = content(self.path) - assert isinstance(content, bytes) - self.wfile.write(content) - - self._port = 2798 # CRYT on telephone keypad - address = ('', self._port) - self._server = HTTPServer(address, RequestHandler) - - @capture_crashes_to_stderr - def bg_task() -> None: - try: - self._server.serve_forever() - finally: - self._server.server_close() - bg_call_later(bg_task, daemon=True) - - def get_url(self, path: str) -> str: - return f'http://localhost:{self._port}' + path - - def close(self) -> None: - self._server.shutdown() - - def __enter__(self) -> 'MockHttpServer': - return self - - def __exit__(self, exc_type, exc_value, exc_traceback) -> None: - self.close() - - # ------------------------------------------------------------------------------ diff --git a/src/crystal/tests/test_new_root_url.py b/src/crystal/tests/test_new_root_url.py index e02de6c5..99143ac5 100644 --- a/src/crystal/tests/test_new_root_url.py +++ b/src/crystal/tests/test_new_root_url.py @@ -3,7 +3,7 @@ from crystal.model import Project, Resource, RootResource from crystal.tests.util.asserts import * from crystal.tests.util.controls import click_button, click_checkbox, TreeItem -from crystal.tests.util.server import served_project +from crystal.tests.util.server import MockHttpServer, served_project from crystal.tests.util.subtests import awith_subtests, SubtestsContext, with_subtests from crystal.tests.util.tasks import wait_for_download_to_start_and_finish from crystal.tests.util.wait import ( diff --git a/src/crystal/tests/util/server.py b/src/crystal/tests/util/server.py index 965821e8..9a7b1588 100644 --- a/src/crystal/tests/util/server.py +++ b/src/crystal/tests/util/server.py @@ -7,16 +7,17 @@ from crystal.server import get_request_url, ProjectServer from crystal.tests.util.runner import bg_fetch_url from crystal.tests.util.wait import DEFAULT_WAIT_TIMEOUT +from crystal.util.bulkheads import capture_crashes_to_stderr from crystal.util import http_date from crystal.util.xdatetime import datetime_is_aware -from crystal.util.xthreading import fg_call_and_wait +from crystal.util.xthreading import bg_call_later, fg_call_and_wait import datetime from email.message import EmailMessage -import json +from http.server import BaseHTTPRequestHandler, HTTPServer import os import re import tempfile -from typing import Dict, Iterator, Optional +from typing import Dict, Iterator, List, Optional import unittest.mock from zipfile import ZipFile @@ -89,6 +90,74 @@ def close_project() -> None: fg_call_and_wait(close_project) +class MockHttpServer: + def __init__(self, routes) -> None: + self.requested_paths = [] # type: List[str] + + mock_server = self # capture + class RequestHandler(BaseHTTPRequestHandler): + def do_GET(self) -> None: # override + mock_server.requested_paths.append(self.path) + + # Look for static route + route = routes.get(self.path) + if route is not None: + self._send_route_response(route) + return + + # Look for dynamic route + for (path_pattern, route) in routes.items(): + if not callable(path_pattern): + continue + if not path_pattern(self.path): + continue + self._send_route_response(route) + return + + self.send_response(404) + self.end_headers() + return + + def _send_route_response(self, route) -> None: + self.send_response(route['status_code']) + for (k, v) in route['headers']: + self.send_header(k, v) + self.end_headers() + + content = route['content'] + if callable(content): + content = content(self.path) + assert isinstance(content, bytes) + self.wfile.write(content) + + self._port = 2798 # CRYT on telephone keypad + address = ('', self._port) + self._server = HTTPServer(address, RequestHandler) + + @capture_crashes_to_stderr + def bg_task() -> None: + try: + self._server.serve_forever() + finally: + self._server.server_close() + bg_call_later(bg_task, daemon=True) + + def get_url(self, path: str) -> str: + return f'http://localhost:{self._port}' + path + + def close(self) -> None: + self._server.shutdown() + + def __enter__(self) -> 'MockHttpServer': + return self + + def __exit__(self, exc_type, exc_value, exc_traceback) -> None: + self.close() + + +# ------------------------------------------------------------------------------ +# Utility: Extracted Project + @contextmanager def extracted_project( zipped_project_filename: str @@ -108,6 +177,9 @@ def extracted_project( yield project_dirpath +# ------------------------------------------------------------------------------ +# Utility: Server Requests + @contextmanager def assert_does_open_webbrowser_to(request_url: str) -> Iterator[None]: with unittest.mock.patch('webbrowser.open', spec=True) as mock_open: From fe834a88b2c4236664658d7ac6c4943ac870eb62 Mon Sep 17 00:00:00 2001 From: David Foster Date: Sun, 21 Jul 2024 12:28:49 -0400 Subject: [PATCH 3/8] New URL Options: Add UI Details: - Looks OK on macOS, Windows 7, Linux - Enables and disables while user types URL - 'Download X Immediately' checkbox has a dynamic label - Tooltip defines a 'static site' --- src/crystal/browser/__init__.py | 19 +++- src/crystal/browser/new_root_url.py | 151 ++++++++++++++++++++++++---- src/crystal/url_input.py | 7 ++ 3 files changed, 154 insertions(+), 23 deletions(-) diff --git a/src/crystal/browser/__init__.py b/src/crystal/browser/__init__.py index a8a46f08..19910560 100644 --- a/src/crystal/browser/__init__.py +++ b/src/crystal/browser/__init__.py @@ -519,12 +519,14 @@ def _on_new_root_url_dialog_ok(self, name: str, url: str, change_prefix_command: ChangePrefixCommand, + download_immediately: bool, + create_group: bool, ) -> None: if url == '': raise ValueError('Invalid blank URL') try: - RootResource(self.project, name, Resource(self.project, url)) + rr = RootResource(self.project, name, Resource(self.project, url)) except RootResource.AlreadyExists: raise ValueError('Invalid duplicate URL') @@ -534,6 +536,16 @@ def _on_new_root_url_dialog_ok(self, self.entity_tree.clear_default_url_prefix() else: self.entity_tree.set_default_url_prefix(*change_prefix_command) + + if create_group: + assert url.endswith('/') + rg = ResourceGroup(self.project, '', url + '**', source=rr) + + if download_immediately: + rg.download(needs_result=False) + else: + if download_immediately: + rr.download(needs_result=False) @fg_affinity def _on_edit_root_url_dialog_ok(self, @@ -541,6 +553,8 @@ def _on_edit_root_url_dialog_ok(self, name: str, url: str, change_prefix_command: ChangePrefixCommand, + download_immediately: bool, + create_group: bool, ) -> None: if url != rr.url: raise ValueError() @@ -558,6 +572,9 @@ def _on_edit_root_url_dialog_ok(self, self.entity_tree.clear_default_url_prefix() else: self.entity_tree.set_default_url_prefix(*change_prefix_command) + + assert download_immediately == False + assert create_group == False # === Entity Pane: New/Edit Group === diff --git a/src/crystal/browser/new_root_url.py b/src/crystal/browser/new_root_url.py index 9e3f3080..e4eabcad 100644 --- a/src/crystal/browser/new_root_url.py +++ b/src/crystal/browser/new_root_url.py @@ -1,4 +1,4 @@ -from crystal.url_input import UrlCleaner +from crystal.url_input import cleaned_url_is_at_site_root, UrlCleaner from crystal.util.ellipsis import Ellipsis, EllipsisType from crystal.util.bulkheads import capture_crashes_to_stderr from crystal.util.wx_bind import bind @@ -6,7 +6,7 @@ CreateButtonSizer, position_dialog_initially, ShowModal, ) from crystal.util.wx_static_box_sizer import wrap_static_box_sizer_child -from crystal.util.xos import is_wx_gtk +from crystal.util.xos import is_linux, is_wx_gtk, is_windows from crystal.util.xthreading import fg_affinity, fg_call_later import os from typing import Callable, Literal, Optional, Tuple, Union @@ -26,7 +26,7 @@ _WINDOW_INNER_PADDING = 10 _FORM_LABEL_INPUT_SPACING = 5 _FORM_ROW_SPACING = 10 -_ABOVE_OPTIONS_PADDING = 10 +_BESIDE_OPTIONS_PADDING = 10 _OPTIONS_SHOWN_LABEL = 'Basic Options' _OPTIONS_NOT_SHOWN_LABEL = 'Advanced Options' @@ -36,6 +36,9 @@ class NewRootUrlDialog: _ID_SET_DOMAIN_PREFIX = 101 _ID_SET_DIRECTORY_PREFIX = 102 + _ID_DOWNLOAD_IMMEDIATELY = 111 + _ID_CREATE_GROUP = 112 + _INITIAL_URL_WIDTH = 400 # in pixels _FIELD_TO_SPINNER_MARGIN = 5 @@ -55,7 +58,7 @@ class NewRootUrlDialog: def __init__(self, parent: wx.Window, - on_finish: Callable[[str, str, ChangePrefixCommand], None], + on_finish: Callable[[str, str, ChangePrefixCommand, bool, bool], None], url_exists_func: Callable[[str], bool], initial_url: str='', initial_name: str='', @@ -100,16 +103,26 @@ def __init__(self, flag=wx.EXPAND|wx.ALL, border=_WINDOW_INNER_PADDING) - self._options_sizer = self._create_advanced_options( + if not is_edit: + new_options_sizer = self._create_new_options(dialog) + dialog_sizer.Add( + new_options_sizer, + flag=wx.EXPAND|wx.LEFT|wx.RIGHT|wx.BOTTOM, + border=self._assert_same( + _WINDOW_INNER_PADDING, # wx.LEFT|wx.RIGHT + _BESIDE_OPTIONS_PADDING, # wx.BOTTOM + )) + + self._advanced_options_sizer = self._create_advanced_options( dialog, initial_set_as_default_domain, initial_set_as_default_directory, allow_set_as_default_domain_or_directory) dialog_sizer.Add( - self._options_sizer, - flag=wx.EXPAND|wx.TOP|wx.LEFT|wx.RIGHT|wx.BOTTOM, + self._advanced_options_sizer, + flag=wx.EXPAND|wx.LEFT|wx.RIGHT|wx.BOTTOM, border=self._assert_same( - _ABOVE_OPTIONS_PADDING, # wx.TOP - _WINDOW_INNER_PADDING, # wx.LEFT|wx.RIGHT|wx.BOTTOM + _WINDOW_INNER_PADDING, # wx.LEFT|wx.RIGHT + _BESIDE_OPTIONS_PADDING, # wx.BOTTOM )) dialog_sizer.Add( @@ -130,12 +143,12 @@ def __init__(self, # TODO: Verify that the wxGTK-specific logic here is actually necessary if not is_wx_gtk(): dialog.Fit() - self._on_options_toggle() # collapse options initially + self._on_advanced_options_toggle() # collapse options initially dialog.Show(True) else: dialog.Show(True) dialog.Fit() # NOTE: Must Fit() after Show() here so that wxGTK actually fits correctly - self._on_options_toggle() # collapse options initially + self._on_advanced_options_toggle() # collapse options initially dialog.MinSize = dialog.Size # TODO: Clamp height to fixed value, but still allow @@ -170,7 +183,7 @@ def _create_fields(self, parent: wx.Window, initial_url: str, initial_name: str, self._url_field.Hint = 'https://example.com/' self._url_field.SetSelection(-1, -1) # select all upon focus self._url_field.Enabled = not is_edit - bind(self._url_field, wx.EVT_TEXT, self._update_ok_enabled) + bind(self._url_field, wx.EVT_TEXT, self._on_url_field_changed) bind(self._url_field, wx.EVT_SET_FOCUS, self._on_url_field_focus) bind(self._url_field, wx.EVT_KILL_FOCUS, self._on_url_field_blur) url_field_and_spinner.Add(self._url_field, proportion=1, flag=wx.EXPAND) @@ -206,6 +219,69 @@ def _create_fields(self, parent: wx.Window, initial_url: str, initial_name: str, return fields_sizer + def _create_new_options(self, parent: wx.Window) -> wx.StaticBoxSizer: + options_sizer = wx.StaticBoxSizer(wx.VERTICAL, parent, label='New URL Options') + options_sizer.Add( + wrap_static_box_sizer_child( + self._create_new_options_content( + options_sizer.GetStaticBox())), + flag=wx.EXPAND) + return options_sizer + + def _create_new_options_content(self, parent: wx.Window) -> wx.Sizer: + options_sizer = wx.BoxSizer(wx.VERTICAL) + + self._download_immediately_checkbox = wx.CheckBox(parent, + id=self._ID_DOWNLOAD_IMMEDIATELY, + label='Download Immediately', + name='cr-new-root-url-dialog__download-immediately-checkbox') + self._download_immediately_checkbox.Value = True + options_sizer.Add(self._download_immediately_checkbox, + flag=wx.BOTTOM, + border=_FORM_LABEL_INPUT_SPACING) + + self._create_group_checkbox = wx.CheckBox(parent, + id=self._ID_CREATE_GROUP, + label='Create Group to Download Entire Site', + name='cr-new-root-url-dialog__create-group-checkbox') + self._create_group_checkbox.Value = False + options_sizer.Add(self._create_group_checkbox, + flag=wx.BOTTOM, + border=5) + + only_simple_sites_label = wx.StaticText(parent, label='') + only_simple_sites_label.ForegroundColour = \ + wx.Colour(102, 102, 102) # gray + only_simple_sites_label.Font = only_simple_sites_label.Font.MakeSmaller() + # NOTE: Underline formatting is not supported on Windows, + # even when using GenStaticText as an alternative to wx.StaticText + only_simple_sites_label.SetLabelMarkup( + 'Only recommended for downloading simple sites') + # NOTE: Altering the label's cursor on wxGTK also seems to set + # the cursor of its enclosing Sizer as a side effect, + # which is confusing. So don't set any cursor on wxGTK. + if not is_wx_gtk(): + only_simple_sites_label.Cursor = wx.Cursor(wx.CURSOR_QUESTION_ARROW) + only_simple_sites_label.ToolTip = ( + 'A simple website is created by a single person only, ' + 'may contain text and images but not video, ' + 'and does not requiring logging in to view its content.' + ) + options_sizer.Add(only_simple_sites_label, + flag=wx.LEFT, + border=( + 17 if is_windows() else + 24 if is_linux() else + 20 # macOS + )) + + bind(parent, wx.EVT_CHECKBOX, self._on_checkbox) + + self._update_create_group_enabled() + self._update_download_immediately_label() + + return options_sizer + def _create_advanced_options(self, parent: wx.Window, *args, **kwargs) -> wx.StaticBoxSizer: options_sizer = wx.StaticBoxSizer(wx.VERTICAL, parent, label='Advanced Options') options_sizer.Add( @@ -296,6 +372,11 @@ def _set_cleaned_url(self, cleaned_url: str) -> None: # === Events === + @fg_affinity + def _on_url_field_changed(self, event=None) -> None: + self._update_create_group_enabled() + self._update_ok_enabled() + # NOTE: Focus event can be called multiple times without an intermediate blur event @fg_affinity def _on_url_field_focus(self, event: wx.FocusEvent) -> None: @@ -364,12 +445,14 @@ def _on_button(self, event: wx.CommandEvent) -> None: elif btn_id == wx.ID_CANCEL: self._on_cancel(event) elif btn_id == wx.ID_MORE: - self._on_options_toggle() + self._on_advanced_options_toggle() @fg_affinity def _on_checkbox(self, event: wx.CommandEvent) -> None: checkbox_id = event.GetEventObject().GetId() - if checkbox_id == self._ID_SET_DOMAIN_PREFIX: + if checkbox_id == self._ID_CREATE_GROUP: + self._update_download_immediately_label() + elif checkbox_id == self._ID_SET_DOMAIN_PREFIX: if self._set_as_default_domain_checkbox.Value: self._set_as_default_directory_checkbox.Value = False elif checkbox_id == self._ID_SET_DIRECTORY_PREFIX: @@ -414,13 +497,21 @@ def _on_ok(self, event: Optional[wx.CommandEvent]=None) -> None: self._ok_button.Enabled = True assert self._cancel_button.Enabled == True return - if self._set_as_default_domain_checkbox.IsChecked(): + if self._set_as_default_domain_checkbox.Value: change_prefix_command = ('domain', url) # type: ChangePrefixCommand - elif self._set_as_default_directory_checkbox.IsChecked(): + elif self._set_as_default_directory_checkbox.Value: change_prefix_command = ('directory', url) else: change_prefix_command = None if self._did_own_prefix else Ellipsis - self._on_finish(name, url, change_prefix_command) + if self._is_edit: + download_immediately = False + create_group = False + else: + download_immediately = self._download_immediately_checkbox.Value + create_group = self._create_group_checkbox.Value + self._on_finish( + name, url, change_prefix_command, + download_immediately, create_group) self._destroy() @@ -434,13 +525,13 @@ def _on_cancel(self, event: wx.CommandEvent) -> None: self._destroy() @fg_affinity - def _on_options_toggle(self) -> None: - options = self._options_sizer.GetStaticBox() + def _on_advanced_options_toggle(self) -> None: + options = self._advanced_options_sizer.GetStaticBox() if options.Shown: # Hide self._options_button.Label = _OPTIONS_NOT_SHOWN_LABEL - options_height = options.Size.Height + _ABOVE_OPTIONS_PADDING + options_height = options.Size.Height + _BESIDE_OPTIONS_PADDING options.Shown = False self.dialog.SetSize( x=wx.DefaultCoord, @@ -453,7 +544,7 @@ def _on_options_toggle(self) -> None: self._options_button.Label = _OPTIONS_SHOWN_LABEL options.Shown = True - options_height = options.Size.Height + _ABOVE_OPTIONS_PADDING + options_height = options.Size.Height + _BESIDE_OPTIONS_PADDING self.dialog.SetSize( x=wx.DefaultCoord, y=wx.DefaultCoord, @@ -472,7 +563,23 @@ def _on_destroyed(self, event) -> None: # === Updates === - def _update_ok_enabled(self, event=None) -> None: + def _update_create_group_enabled(self) -> None: + url = self._url_field.Value + self._create_group_checkbox.Enabled = ( + url == '' or cleaned_url_is_at_site_root(url) + ) + + self._update_download_immediately_label() + + def _update_download_immediately_label(self) -> None: + self._download_immediately_checkbox.Label = ( + 'Download Site Immediately' if ( + self._create_group_checkbox.Enabled and + self._create_group_checkbox.Value + ) else 'Download URL Immediately' + ) + + def _update_ok_enabled(self) -> None: self._ok_button.Enabled = (self._url_field.Value != '') diff --git a/src/crystal/url_input.py b/src/crystal/url_input.py index 5c624143..f200aebf 100644 --- a/src/crystal/url_input.py +++ b/src/crystal/url_input.py @@ -80,6 +80,13 @@ def cancel(self) -> None: self._finish(None) +def cleaned_url_is_at_site_root(url_input: str) -> bool: + candidates = _candidate_urls_from_user_input(url_input) + assert len(candidates) >= 1 + candidate = candidates[0] + return urlparse(candidate).path == '/' + + def _candidate_urls_from_user_input(url_input: str) -> List[str]: """ Given a potentially messy URL typed manually by the user, From 1ee702429cb433f488491cbaadc16cb8b4764beb Mon Sep 17 00:00:00 2001 From: David Foster Date: Wed, 21 Aug 2024 20:57:40 -0400 Subject: [PATCH 4/8] New URL Options: Add new tests --- src/crystal/browser/new_root_url.py | 3 + src/crystal/tests/test_new_root_url.py | 166 ++++++++++++++++++++++++- src/crystal/tests/util/windows.py | 25 ++++ 3 files changed, 192 insertions(+), 2 deletions(-) diff --git a/src/crystal/browser/new_root_url.py b/src/crystal/browser/new_root_url.py index e4eabcad..fb0a94e7 100644 --- a/src/crystal/browser/new_root_url.py +++ b/src/crystal/browser/new_root_url.py @@ -51,6 +51,9 @@ class NewRootUrlDialog: _cancel_button: wx.Button _options_button: wx.Button + _download_immediately_checkbox: wx.CheckBox + _create_group_checkbox: wx.CheckBox + _set_as_default_domain_checkbox: wx.CheckBox _set_as_default_directory_checkbox: wx.CheckBox diff --git a/src/crystal/tests/test_new_root_url.py b/src/crystal/tests/test_new_root_url.py index 99143ac5..3980fa76 100644 --- a/src/crystal/tests/test_new_root_url.py +++ b/src/crystal/tests/test_new_root_url.py @@ -1,14 +1,22 @@ from contextlib import asynccontextmanager, contextmanager from crystal.browser.new_root_url import fields_hide_hint_when_focused from crystal.model import Project, Resource, RootResource +from crystal.task import DownloadResourceGroupTask +from crystal.tests.test_download import MockHttpServer # TODO: extract to shared module from crystal.tests.util.asserts import * from crystal.tests.util.controls import click_button, click_checkbox, TreeItem from crystal.tests.util.server import MockHttpServer, served_project from crystal.tests.util.subtests import awith_subtests, SubtestsContext, with_subtests -from crystal.tests.util.tasks import wait_for_download_to_start_and_finish +from crystal.tests.util.tasks import ( + append_deferred_top_level_tasks, + clear_top_level_tasks_on_exit, + scheduler_disabled, + wait_for_download_to_start_and_finish, +) from crystal.tests.util.wait import ( DEFAULT_WAIT_PERIOD, first_child_of_tree_item_is_not_loading_condition, + tree_has_no_children_condition, wait_for, ) from crystal.tests.util.windows import NewRootUrlDialog, EntityTree, OpenOrCreateDialog @@ -19,7 +27,7 @@ import os import tempfile import time -from typing import AsyncIterator, Dict, Iterator, List, Optional, Union +from typing import AsyncIterator, Dict, Iterator, List, Optional, Tuple, Union from typing_extensions import Self from unittest import skip from unittest.mock import ANY, patch @@ -211,6 +219,160 @@ async def test_given_resource_node_with_link_labeled_as_root_url_can_easily_forg pass +# === Test: New URL Options === + +async def test_when_add_url_then_downloads_url_immediately_by_default() -> None: + with _served_simple_site() as (home_url, _): + async with (await OpenOrCreateDialog.wait_for()).create() as (mw, _): + project = Project._last_opened_project + assert project is not None + + assert mw.new_root_url_button.Enabled + click_button(mw.new_root_url_button) + nud = await NewRootUrlDialog.wait_for() + + assert nud.download_immediately_checkbox.Value + + nud.url_field.Value = home_url + await nud.ok() + + # Ensure started downloading + root_ti = TreeItem.GetRootItem(mw.entity_tree.window) + home_ti = root_ti.find_child(home_url, project.default_url_prefix) + await wait_for_download_to_start_and_finish(mw.task_tree) + await _assert_tree_item_icon_tooltip_contains(home_ti, 'Fresh') + + +async def test_when_add_url_then_can_avoid_downloading_url_with_1_extra_click() -> None: + with _served_simple_site() as (home_url, _): + async with (await OpenOrCreateDialog.wait_for()).create() as (mw, _): + project = Project._last_opened_project + assert project is not None + + assert mw.new_root_url_button.Enabled + click_button(mw.new_root_url_button) + nud = await NewRootUrlDialog.wait_for() + + nud.url_field.Value = home_url + click_checkbox(nud.download_immediately_checkbox) # extra click #1 + assert not nud.download_immediately_checkbox.Value + await nud.ok() + + # Ensure did NOT start downloading + root_ti = TreeItem.GetRootItem(mw.entity_tree.window) + home_ti = root_ti.find_child(home_url, project.default_url_prefix) + assert tree_has_no_children_condition(mw.task_tree)() + await _assert_tree_item_icon_tooltip_contains(home_ti, 'Undownloaded') + + +async def test_when_add_url_at_site_root_then_can_download_site_with_1_extra_click() -> None: + with _served_simple_site() as (home_url, _), scheduler_disabled(): + async with (await OpenOrCreateDialog.wait_for()).create() as (mw, _): + project = Project._last_opened_project + assert project is not None + + with clear_top_level_tasks_on_exit(project): + assert mw.new_root_url_button.Enabled + click_button(mw.new_root_url_button) + nud = await NewRootUrlDialog.wait_for() + + nud.url_field.Value = home_url # at site root + assert nud.download_immediately_checkbox.Value + assert nud.create_group_checkbox.Enabled + click_checkbox(nud.create_group_checkbox) # extra click #1 + assert nud.create_group_checkbox.Value + await nud.ok() + append_deferred_top_level_tasks(project) + + root_ti = TreeItem.GetRootItem(mw.entity_tree.window) + home_ti = root_ti.find_child(home_url, project.default_url_prefix) + site_g = root_ti.find_child(home_url + '**', project.default_url_prefix) + (download_rg_task,) = project.root_task.children + assert isinstance(download_rg_task, DownloadResourceGroupTask) + # (Do NOT wait for group to finish downloading) + + +async def test_when_add_url_not_at_site_root_then_cannot_download_site_or_create_group_for_site() -> None: + with _served_simple_site() as (home_url, image_url): + async with (await OpenOrCreateDialog.wait_for()).create() as (mw, _): + project = Project._last_opened_project + assert project is not None + + assert mw.new_root_url_button.Enabled + click_button(mw.new_root_url_button) + nud = await NewRootUrlDialog.wait_for() + + nud.url_field.Value = image_url # NOT at site root + assert not nud.create_group_checkbox.Enabled + await nud.cancel() + + +async def test_when_add_url_at_site_root_then_can_create_group_for_site_but_not_download_it_with_extra_clicks() -> None: + with _served_simple_site() as (home_url, _), scheduler_disabled(): + async with (await OpenOrCreateDialog.wait_for()).create() as (mw, _): + project = Project._last_opened_project + assert project is not None + + with clear_top_level_tasks_on_exit(project): + assert mw.new_root_url_button.Enabled + click_button(mw.new_root_url_button) + nud = await NewRootUrlDialog.wait_for() + + nud.url_field.Value = home_url # at site root + assert nud.create_group_checkbox.Enabled + click_checkbox(nud.create_group_checkbox) # extra click #1 + assert nud.create_group_checkbox.Value + click_checkbox(nud.download_immediately_checkbox) # extra click #2 + assert not nud.download_immediately_checkbox.Value + await nud.ok() + append_deferred_top_level_tasks(project) + + root_ti = TreeItem.GetRootItem(mw.entity_tree.window) + home_ti = root_ti.find_child(home_url, project.default_url_prefix) + site_g = root_ti.find_child(home_url + '**', project.default_url_prefix) + () = project.root_task.children + + +async def test_when_edit_url_then_new_url_options_not_shown() -> None: + with _served_simple_site() as (home_url, _): + async with (await OpenOrCreateDialog.wait_for()).create() as (mw, _): + project = Project._last_opened_project + assert project is not None + + rr = RootResource(project, 'Home', Resource(project, home_url)) + root_ti = TreeItem.GetRootItem(mw.entity_tree.window) + home_ti = root_ti.find_child(home_url, project.default_url_prefix) + home_ti.SelectItem() + + assert mw.edit_button.Enabled + click_button(mw.edit_button) + nud = await NewRootUrlDialog.wait_for() + + assert not nud.new_options_shown + await nud.cancel() + + +@contextmanager +def _served_simple_site() -> Iterator[Tuple[str, str]]: + server = MockHttpServer({ + '/': dict( + status_code=200, + headers=[('Content-Type', 'text/html')], + content=''.encode('utf-8') + ), + '/assets/image.png': dict( + status_code=200, + headers=[('Content-Type', 'image/png')], + content=b'' + ) + }) + with server: + home_url = server.get_url('/') + image_url = server.get_url('/assets/image.png') + + yield (home_url, image_url) + + # === Test: Default URL Prefix: Load/Save === async def test_when_new_url_and_save_given_project_prefix_is_unset_then_sets_prefix_to_domain() -> None: diff --git a/src/crystal/tests/util/windows.py b/src/crystal/tests/util/windows.py index f8d83065..3b3abdd3 100644 --- a/src/crystal/tests/util/windows.py +++ b/src/crystal/tests/util/windows.py @@ -333,6 +333,9 @@ class NewRootUrlDialog: ok_button: wx.Button cancel_button: wx.Button + download_immediately_checkbox: wx.CheckBox # or None + create_group_checkbox: wx.CheckBox # or None + options_button: wx.Button set_as_default_domain_checkbox: wx.CheckBox set_as_default_directory_checkbox: wx.CheckBox @@ -356,12 +359,25 @@ async def wait_for() -> NewRootUrlDialog: assert isinstance(self.ok_button, wx.Button) self.cancel_button = self._dialog.FindWindow(id=wx.ID_CANCEL) assert isinstance(self.cancel_button, wx.Button) + + self.download_immediately_checkbox = self._dialog.FindWindow(name='cr-new-root-url-dialog__download-immediately-checkbox') + assert ( + self.download_immediately_checkbox is None or + isinstance(self.download_immediately_checkbox, wx.CheckBox) + ) + self.create_group_checkbox = self._dialog.FindWindow(name='cr-new-root-url-dialog__create-group-checkbox') + assert ( + self.create_group_checkbox is None or + isinstance(self.create_group_checkbox, wx.CheckBox) + ) + self.options_button = self._dialog.FindWindow(id=wx.ID_MORE) assert isinstance(self.options_button, wx.Button) self.set_as_default_domain_checkbox = self._dialog.FindWindow(name='cr-new-root-url-dialog__set-as-default-domain-checkbox') assert isinstance(self.set_as_default_domain_checkbox, wx.CheckBox) self.set_as_default_directory_checkbox = self._dialog.FindWindow(name='cr-new-root-url-dialog__set-as-default-directory-checkbox') assert isinstance(self.set_as_default_directory_checkbox, wx.CheckBox) + return self def __init__(self, *, ready: bool=False) -> None: @@ -377,6 +393,13 @@ def shown(self) -> bool: def url_field_focused(self) -> bool: return self._controller._url_field_focused + @property + def new_options_shown(self) -> bool: + return ( + self.download_immediately_checkbox is not None and + self.create_group_checkbox is not None + ) + async def ok(self) -> None: click_button(self.ok_button) await wait_for(not_condition(window_condition('cr-new-root-url-dialog')), stacklevel_extra=1) @@ -385,6 +408,8 @@ async def cancel(self) -> None: click_button(self.cancel_button) await wait_for(not_condition(window_condition('cr-new-root-url-dialog')), stacklevel_extra=1) + # === Utility === + def do_not_set_default_url_prefix(self) -> None: """ Configures the URL being created to NOT also set it as the default domain. From 6fe59e5f73f234b548e92a122d30bf62ed4c282c Mon Sep 17 00:00:00 2001 From: David Foster Date: Thu, 22 Aug 2024 21:48:18 -0400 Subject: [PATCH 5/8] New URL Options: Update preexisting tests --- src/crystal/tests/test_new_group.py | 2 ++ src/crystal/tests/test_new_root_url.py | 12 ++++++++++++ src/crystal/tests/test_server.py | 1 + src/crystal/tests/test_workflows.py | 8 ++++++++ src/crystal/tests/util/windows.py | 15 +++++++++++++++ 5 files changed, 38 insertions(+) diff --git a/src/crystal/tests/test_new_group.py b/src/crystal/tests/test_new_group.py index 783744b8..eb780a3f 100644 --- a/src/crystal/tests/test_new_group.py +++ b/src/crystal/tests/test_new_group.py @@ -44,6 +44,7 @@ async def test_can_create_group_with_source( nud.name_field.Value = 'Home' nud.url_field.Value = home_url + nud.do_not_download_immediately() nud.do_not_set_default_url_prefix() await nud.ok() @@ -186,6 +187,7 @@ async def test_given_resource_node_with_multiple_link_children_matching_url_patt nud.name_field.Value = 'Home' nud.url_field.Value = home_url + nud.do_not_download_immediately() await nud.ok() (home_ti,) = root_ti.Children diff --git a/src/crystal/tests/test_new_root_url.py b/src/crystal/tests/test_new_root_url.py index 3980fa76..9a83d232 100644 --- a/src/crystal/tests/test_new_root_url.py +++ b/src/crystal/tests/test_new_root_url.py @@ -57,6 +57,7 @@ async def test_can_create_root_url( assert mw.new_root_url_button.Enabled click_button(mw.new_root_url_button) nud = await NewRootUrlDialog.wait_for() + nud.do_not_download_immediately() # Ensure prepopulates reasonable information assert '' == nud.url_field.Value @@ -110,6 +111,7 @@ async def test_can_create_root_url( # Recreate the root URL click_button(mw.new_root_url_button) nud = await NewRootUrlDialog.wait_for() + nud.do_not_download_immediately() nud.name_field.Value = 'Home' nud.url_field.Value = home_url await nud.ok() @@ -157,6 +159,7 @@ async def test_given_resource_node_with_links_can_create_new_root_url_to_label_l assert mw.new_root_url_button.Enabled click_button(mw.new_root_url_button) nud = await NewRootUrlDialog.wait_for() + nud.do_not_download_immediately() nud.name_field.Value = 'Home' nud.url_field.Value = home_url @@ -177,6 +180,7 @@ async def test_given_resource_node_with_links_can_create_new_root_url_to_label_l assert mw.new_root_url_button.Enabled click_button(mw.new_root_url_button) nud = await NewRootUrlDialog.wait_for() + nud.do_not_download_immediately() # Ensure prepopulates reasonable information assert atom_feed_url == nud.url_field.Value # default pattern = (from resource) @@ -386,6 +390,7 @@ async def test_when_new_url_and_save_given_project_prefix_is_unset_then_sets_pre assert mw.new_root_url_button.Enabled click_button(mw.new_root_url_button) nud = await NewRootUrlDialog.wait_for() + nud.do_not_download_immediately() nud.url_field.Value = 'https://xkcd.com/' await nud.ok() @@ -401,6 +406,7 @@ async def test_when_new_url_and_save_given_project_prefix_is_unset_then_sets_pre assert mw.new_root_url_button.Enabled click_button(mw.new_root_url_button) nud = await NewRootUrlDialog.wait_for() + nud.do_not_download_immediately() nud.url_field.Value = 'mailto:me@example.com' await nud.ok() @@ -426,6 +432,7 @@ async def test_when_new_url_and_save_given_project_prefix_is_set_then_maintains_ assert mw.new_root_url_button.Enabled click_button(mw.new_root_url_button) nud = await NewRootUrlDialog.wait_for() + nud.do_not_download_immediately() nud.url_field.Value = 'https://xkcd.com/' await nud.ok() @@ -495,6 +502,7 @@ async def test_when_new_url_and_set_prefix_to_x_and_save_then_sets_prefix_to_x() assert mw.new_root_url_button.Enabled click_button(mw.new_root_url_button) nud = await NewRootUrlDialog.wait_for() + nud.do_not_download_immediately() nud.url_field.Value = 'https://neocities.org/' @@ -517,6 +525,7 @@ async def test_when_new_url_and_set_prefix_to_x_and_save_then_sets_prefix_to_x() assert mw.new_root_url_button.Enabled click_button(mw.new_root_url_button) nud = await NewRootUrlDialog.wait_for() + nud.do_not_download_immediately() nud.url_field.Value = 'https://neocities.org/~distantskies/' @@ -546,6 +555,7 @@ async def test_when_new_url_and_set_prefix_to_x_and_save_then_sets_prefix_to_x() assert mw.new_root_url_button.Enabled click_button(mw.new_root_url_button) nud = await NewRootUrlDialog.wait_for() + nud.do_not_download_immediately() nud.url_field.Value = 'https://neocities.org/' @@ -568,6 +578,7 @@ async def test_when_new_url_and_set_prefix_to_x_and_save_then_sets_prefix_to_x() assert mw.new_root_url_button.Enabled click_button(mw.new_root_url_button) nud = await NewRootUrlDialog.wait_for() + nud.do_not_download_immediately() nud.url_field.Value = 'https://neocities.org/~distantskies/' @@ -1223,6 +1234,7 @@ async def _new_root_url_dialog_open(*, autoclose: bool=True) -> AsyncIterator[Ne async with (await OpenOrCreateDialog.wait_for()).create() as (mw, _): click_button(mw.new_root_url_button) nud = await NewRootUrlDialog.wait_for() + nud.do_not_download_immediately() try: yield nud diff --git a/src/crystal/tests/test_server.py b/src/crystal/tests/test_server.py index a9291646..df5eb52e 100644 --- a/src/crystal/tests/test_server.py +++ b/src/crystal/tests/test_server.py @@ -55,6 +55,7 @@ async def test_given_default_serving_port_in_use_when_start_serving_project_then nud.name_field.Value = 'Home' nud.url_field.Value = home_url + nud.do_not_download_immediately() await nud.ok() (home_ti,) = root_ti.Children diff --git a/src/crystal/tests/test_workflows.py b/src/crystal/tests/test_workflows.py index 922d9519..d12f920d 100644 --- a/src/crystal/tests/test_workflows.py +++ b/src/crystal/tests/test_workflows.py @@ -74,6 +74,7 @@ async def test_can_download_and_serve_a_static_site() -> None: nud.name_field.Value = 'Home' nud.url_field.Value = home_url + nud.do_not_download_immediately() nud.do_not_set_default_url_prefix() await nud.ok() home_ti = root_ti.GetFirstChild() @@ -390,6 +391,7 @@ async def test_can_download_and_serve_a_site_requiring_dynamic_url_discovery() - nud = await NewRootUrlDialog.wait_for() nud.name_field.Value = 'Home' nud.url_field.Value = home_url + nud.do_not_download_immediately() nud.do_not_set_default_url_prefix() await nud.ok() home_ti = root_ti.GetFirstChild() @@ -531,6 +533,7 @@ def start_server_again(): nud = await NewRootUrlDialog.wait_for() nud.name_field.Value = target_root_resource_name nud.url_field.Value = target_url + nud.do_not_download_immediately() nud.do_not_set_default_url_prefix() await nud.ok() @@ -591,6 +594,7 @@ async def test_can_download_and_serve_a_site_requiring_dynamic_link_rewriting() nud = await NewRootUrlDialog.wait_for() nud.name_field.Value = 'Home' nud.url_field.Value = home_url + nud.do_not_download_immediately() await nud.ok() home_ti = root_ti.GetFirstChild() assert home_ti is not None # entity was created @@ -705,6 +709,7 @@ async def test_cannot_download_anything_given_project_is_opened_as_readonly() -> nud = await NewRootUrlDialog.wait_for() nud.name_field.Value = 'Home' nud.url_field.Value = home_url + nud.do_not_download_immediately() nud.do_not_set_default_url_prefix() await nud.ok() home_ti = root_ti.GetFirstChild() @@ -814,12 +819,14 @@ async def test_can_update_downloaded_site_with_newer_page_revisions() -> None: nud = await NewRootUrlDialog.wait_for() nud.name_field.Value = 'Home' nud.url_field.Value = home_url + nud.do_not_download_immediately() await nud.ok() click_button(mw.new_root_url_button) nud = await NewRootUrlDialog.wait_for() nud.name_field.Value = 'Comic #1' nud.url_field.Value = comic1_url + nud.do_not_download_immediately() await nud.ok() root_ti = TreeItem.GetRootItem(mw.entity_tree.window) @@ -981,6 +988,7 @@ async def test_can_download_a_static_site_with_unnamed_root_urls_and_groups() -> nud = await NewRootUrlDialog.wait_for() nud.url_field.Value = home_url + nud.do_not_download_immediately() nud.do_not_set_default_url_prefix() await nud.ok() home_ti = root_ti.GetFirstChild() diff --git a/src/crystal/tests/util/windows.py b/src/crystal/tests/util/windows.py index 3b3abdd3..c22e657c 100644 --- a/src/crystal/tests/util/windows.py +++ b/src/crystal/tests/util/windows.py @@ -410,6 +410,21 @@ async def cancel(self) -> None: # === Utility === + def do_not_download_immediately(self) -> None: + """ + Configures the URL being created so that it is NOT immediately downloaded + after creation. + + Several tests that create a URL are not interested in the default + "download immediately" behavior and are simpler to write when there + is no need to worry about or clean up after a URL is downloaded as + a side effect of creating it. + """ + if self.download_immediately_checkbox is None: + return + if self.download_immediately_checkbox.Value: + self.download_immediately_checkbox.Value = False + def do_not_set_default_url_prefix(self) -> None: """ Configures the URL being created to NOT also set it as the default domain. From 8be0ac5efc565275a014cf86f6b878fccbb424c4 Mon Sep 17 00:00:00 2001 From: David Foster Date: Sun, 25 Aug 2024 09:00:23 -0400 Subject: [PATCH 6/8] New Group Options: Add UI --- src/crystal/browser/__init__.py | 7 +++++ src/crystal/browser/new_group.py | 46 ++++++++++++++++++++++++++++---- 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/src/crystal/browser/__init__.py b/src/crystal/browser/__init__.py index 19910560..7f0e6afa 100644 --- a/src/crystal/browser/__init__.py +++ b/src/crystal/browser/__init__.py @@ -597,6 +597,7 @@ def _on_new_group_dialog_ok(self, url_pattern: str, source: ResourceGroupSource, do_not_download: bool, + download_immediately: bool, ) -> None: # TODO: Validate user input: # * Is url_pattern empty? @@ -604,6 +605,9 @@ def _on_new_group_dialog_ok(self, rg = ResourceGroup( self.project, name, url_pattern, source, do_not_download=do_not_download) + + if download_immediately: + rg.download(needs_result=False) @fg_affinity def _on_edit_group_dialog_ok(self, @@ -612,6 +616,7 @@ def _on_edit_group_dialog_ok(self, url_pattern: str, source: ResourceGroupSource, do_not_download: bool, + download_immediately: bool, ) -> None: if url_pattern != rg.url_pattern: raise ValueError() @@ -620,6 +625,8 @@ def _on_edit_group_dialog_ok(self, # TODO: This update should happen in response to an event # fired by the entity itself. self.entity_tree.root.update_title_of_descendants() # update names in titles + + assert download_immediately == False def _saving_source_would_create_cycle(self, rg: ResourceGroup, source: ResourceGroupSource) -> bool: ancestor_source = source # type: ResourceGroupSource diff --git a/src/crystal/browser/new_group.py b/src/crystal/browser/new_group.py index 8a703c89..9d922684 100644 --- a/src/crystal/browser/new_group.py +++ b/src/crystal/browser/new_group.py @@ -35,7 +35,7 @@ class NewGroupDialog: def __init__(self, parent: wx.Window, - on_finish: Callable[[str, str, ResourceGroupSource, bool], None], + on_finish: Callable[[str, str, ResourceGroupSource, bool, bool], None], project: Project, saving_source_would_create_cycle_func: Callable[[ResourceGroupSource], bool], initial_url_pattern: str='', @@ -59,6 +59,7 @@ def __init__(self, self._project = project self._on_finish = on_finish self._saving_source_would_create_cycle_func = saving_source_would_create_cycle_func + self._is_edit = is_edit # Show progress dialog in advance if will need to load all project URLs try: @@ -91,9 +92,16 @@ def __init__(self, flag=wx.EXPAND|preview_box_flags, border=preview_box_border) - self._options_sizer = self._create_advanced_options(dialog, initial_do_not_download) + if not is_edit: + new_options_sizer = self._create_new_options(dialog) + content_sizer.Add( + new_options_sizer, + flag=wx.EXPAND|wx.TOP, + border=_ABOVE_OPTIONS_PADDING) + + self._advanced_options_sizer = self._create_advanced_options(dialog, initial_do_not_download) content_sizer.Add( - self._options_sizer, + self._advanced_options_sizer, flag=wx.EXPAND|wx.TOP, border=_ABOVE_OPTIONS_PADDING) @@ -238,6 +246,28 @@ def _create_preview_box_content(self, parent: wx.Window) -> wx.Sizer: return content_sizer + def _create_new_options(self, parent: wx.Window) -> wx.StaticBoxSizer: + options_sizer = wx.StaticBoxSizer(wx.VERTICAL, parent, label='New Group Options') + options_sizer.Add( + wrap_static_box_sizer_child( + self._create_new_options_content( + options_sizer.GetStaticBox())), + flag=wx.EXPAND) + return options_sizer + + def _create_new_options_content(self, parent: wx.Window) -> wx.Sizer: + options_sizer = wx.BoxSizer(wx.VERTICAL) + + self._download_immediately_checkbox = wx.CheckBox(parent, + label='Download Group Immediately', + name='cr-new-group-dialog__download-immediately-checkbox') + self._download_immediately_checkbox.Value = False + options_sizer.Add(self._download_immediately_checkbox, + flag=wx.BOTTOM, + border=_FORM_LABEL_INPUT_SPACING) + + return options_sizer + def _create_advanced_options(self, parent: wx.Window, *args, **kwargs) -> wx.StaticBoxSizer: options_sizer = wx.StaticBoxSizer(wx.VERTICAL, parent, label='Advanced Options') options_sizer.Add( @@ -455,7 +485,13 @@ def _on_ok(self) -> None: assert wx.ID_OK == choice return do_not_download = self.do_not_download_checkbox.Value - self._on_finish(name, url_pattern, source, do_not_download) + if self._is_edit: + download_immediately = False + else: + download_immediately = self._download_immediately_checkbox.Value + self._on_finish( + name, url_pattern, source, do_not_download, + download_immediately) self.dialog.Destroy() @@ -465,7 +501,7 @@ def _on_cancel(self) -> None: @fg_affinity def _on_options_toggle(self) -> None: - options = self._options_sizer.GetStaticBox() + options = self._advanced_options_sizer.GetStaticBox() if options.Shown: # Hide self._options_button.Label = _OPTIONS_NOT_SHOWN_LABEL From 1f895f335fc7c7265249136eb7e49479f3dc2281 Mon Sep 17 00:00:00 2001 From: David Foster Date: Sun, 25 Aug 2024 09:19:43 -0400 Subject: [PATCH 7/8] New Group Options: Add new tests --- src/crystal/tests/test_new_group.py | 99 +++++++++++++++++++++++++- src/crystal/tests/test_new_root_url.py | 15 ++-- src/crystal/tests/util/windows.py | 15 ++++ 3 files changed, 118 insertions(+), 11 deletions(-) diff --git a/src/crystal/tests/test_new_group.py b/src/crystal/tests/test_new_group.py index eb780a3f..aaf10666 100644 --- a/src/crystal/tests/test_new_group.py +++ b/src/crystal/tests/test_new_group.py @@ -1,12 +1,14 @@ +from contextlib import contextmanager from crystal.model import Project, Resource, RootResource, ResourceGroup from crystal.tests.util.asserts import assertEqual -from crystal.tests.util.controls import click_button, TreeItem -from crystal.tests.util.server import served_project +from crystal.tests.util.controls import click_button, click_checkbox, TreeItem +from crystal.tests.util.server import MockHttpServer, served_project from crystal.tests.util.ssd import database_on_ssd from crystal.tests.util.tasks import wait_for_download_to_start_and_finish from crystal.tests.util.wait import ( first_child_of_tree_item_is_not_loading_condition, tree_item_has_no_children_condition, + tree_has_no_children_condition, wait_for, ) from crystal.tests.util.windows import ( @@ -15,7 +17,7 @@ from crystal.tests.util.xurlparse import urlpatternparse from crystal.util.xos import is_windows import re -from typing import Optional +from typing import Iterator, Optional, Tuple from unittest import skip from unittest.mock import patch import wx @@ -533,6 +535,97 @@ async def test_given_urls_loaded_and_new_url_created_when_show_new_group_dialog_ await ngd.cancel() +# === Test: New Group Options === + +async def test_when_add_group_then_does_not_download_immediately_by_default() -> None: + with _served_simple_site_with_group() as (home_url, comic_pattern): + async with (await OpenOrCreateDialog.wait_for()).create() as (mw, _): + project = Project._last_opened_project + assert project is not None + + rr = RootResource(project, 'Home', Resource(project, home_url)) + rr.download() + await wait_for_download_to_start_and_finish(mw.task_tree) + + assert mw.new_group_button.Enabled + click_button(mw.new_group_button) + ngd = await NewGroupDialog.wait_for() + + ngd.pattern_field.Value = comic_pattern + ngd.name_field.Value = 'Comic' + await ngd.ok() + + # Ensure did NOT start downloading + root_ti = TreeItem.GetRootItem(mw.entity_tree.window) + comic_ti = root_ti.find_child(comic_pattern, project.default_url_prefix) + assert tree_has_no_children_condition(mw.task_tree)() + + +async def test_when_add_group_can_download_group_immediately_with_1_extra_click() -> None: + with _served_simple_site_with_group() as (home_url, comic_pattern): + async with (await OpenOrCreateDialog.wait_for()).create() as (mw, _): + project = Project._last_opened_project + assert project is not None + + rr = RootResource(project, 'Home', Resource(project, home_url)) + rr.download() + await wait_for_download_to_start_and_finish(mw.task_tree) + + assert mw.new_group_button.Enabled + click_button(mw.new_group_button) + ngd = await NewGroupDialog.wait_for() + + ngd.pattern_field.Value = comic_pattern + ngd.name_field.Value = 'Comic' + click_checkbox(ngd.download_immediately_checkbox) # extra click #1 + await ngd.ok() + + # Ensure started downloading + root_ti = TreeItem.GetRootItem(mw.entity_tree.window) + comic_ti = root_ti.find_child(comic_pattern, project.default_url_prefix) + await wait_for_download_to_start_and_finish(mw.task_tree) + + +async def test_when_edit_group_then_new_group_options_not_shown() -> None: + with _served_simple_site_with_group() as (_, comic_pattern): + async with (await OpenOrCreateDialog.wait_for()).create() as (mw, _): + project = Project._last_opened_project + assert project is not None + + rg = ResourceGroup(project, 'Comic', comic_pattern) + root_ti = TreeItem.GetRootItem(mw.entity_tree.window) + comic_ti = root_ti.find_child(comic_pattern, project.default_url_prefix) + comic_ti.SelectItem() + + assert mw.edit_button.Enabled + click_button(mw.edit_button) + ngd = await NewGroupDialog.wait_for() + + assert not ngd.new_options_shown + await ngd.cancel() + + +@contextmanager +def _served_simple_site_with_group() -> Iterator[Tuple[str, str]]: + server = MockHttpServer({ + '/': dict( + status_code=200, + headers=[('Content-Type', 'text/html')], + content='Comic 1'.encode('utf-8') + ), + '/assets/image.png': dict( + status_code=200, + headers=[('Content-Type', 'image/png')], + content=b'' + ) + }) + with server: + home_url = server.get_url('/') + comic_pattern = server.get_url('/**') + + yield (home_url, comic_pattern) + + # === Utility === _find_child = TreeItem.find_child diff --git a/src/crystal/tests/test_new_root_url.py b/src/crystal/tests/test_new_root_url.py index 9a83d232..95218add 100644 --- a/src/crystal/tests/test_new_root_url.py +++ b/src/crystal/tests/test_new_root_url.py @@ -2,7 +2,6 @@ from crystal.browser.new_root_url import fields_hide_hint_when_focused from crystal.model import Project, Resource, RootResource from crystal.task import DownloadResourceGroupTask -from crystal.tests.test_download import MockHttpServer # TODO: extract to shared module from crystal.tests.util.asserts import * from crystal.tests.util.controls import click_button, click_checkbox, TreeItem from crystal.tests.util.server import MockHttpServer, served_project @@ -226,7 +225,7 @@ async def test_given_resource_node_with_link_labeled_as_root_url_can_easily_forg # === Test: New URL Options === async def test_when_add_url_then_downloads_url_immediately_by_default() -> None: - with _served_simple_site() as (home_url, _): + with _served_simple_site_with_2_urls() as (home_url, _): async with (await OpenOrCreateDialog.wait_for()).create() as (mw, _): project = Project._last_opened_project assert project is not None @@ -248,7 +247,7 @@ async def test_when_add_url_then_downloads_url_immediately_by_default() -> None: async def test_when_add_url_then_can_avoid_downloading_url_with_1_extra_click() -> None: - with _served_simple_site() as (home_url, _): + with _served_simple_site_with_2_urls() as (home_url, _): async with (await OpenOrCreateDialog.wait_for()).create() as (mw, _): project = Project._last_opened_project assert project is not None @@ -270,7 +269,7 @@ async def test_when_add_url_then_can_avoid_downloading_url_with_1_extra_click() async def test_when_add_url_at_site_root_then_can_download_site_with_1_extra_click() -> None: - with _served_simple_site() as (home_url, _), scheduler_disabled(): + with _served_simple_site_with_2_urls() as (home_url, _), scheduler_disabled(): async with (await OpenOrCreateDialog.wait_for()).create() as (mw, _): project = Project._last_opened_project assert project is not None @@ -297,7 +296,7 @@ async def test_when_add_url_at_site_root_then_can_download_site_with_1_extra_cli async def test_when_add_url_not_at_site_root_then_cannot_download_site_or_create_group_for_site() -> None: - with _served_simple_site() as (home_url, image_url): + with _served_simple_site_with_2_urls() as (home_url, image_url): async with (await OpenOrCreateDialog.wait_for()).create() as (mw, _): project = Project._last_opened_project assert project is not None @@ -312,7 +311,7 @@ async def test_when_add_url_not_at_site_root_then_cannot_download_site_or_create async def test_when_add_url_at_site_root_then_can_create_group_for_site_but_not_download_it_with_extra_clicks() -> None: - with _served_simple_site() as (home_url, _), scheduler_disabled(): + with _served_simple_site_with_2_urls() as (home_url, _), scheduler_disabled(): async with (await OpenOrCreateDialog.wait_for()).create() as (mw, _): project = Project._last_opened_project assert project is not None @@ -338,7 +337,7 @@ async def test_when_add_url_at_site_root_then_can_create_group_for_site_but_not_ async def test_when_edit_url_then_new_url_options_not_shown() -> None: - with _served_simple_site() as (home_url, _): + with _served_simple_site_with_2_urls() as (home_url, _): async with (await OpenOrCreateDialog.wait_for()).create() as (mw, _): project = Project._last_opened_project assert project is not None @@ -357,7 +356,7 @@ async def test_when_edit_url_then_new_url_options_not_shown() -> None: @contextmanager -def _served_simple_site() -> Iterator[Tuple[str, str]]: +def _served_simple_site_with_2_urls() -> Iterator[Tuple[str, str]]: server = MockHttpServer({ '/': dict( status_code=200, diff --git a/src/crystal/tests/util/windows.py b/src/crystal/tests/util/windows.py index c22e657c..c7373111 100644 --- a/src/crystal/tests/util/windows.py +++ b/src/crystal/tests/util/windows.py @@ -453,6 +453,8 @@ class NewGroupDialog: cancel_button: wx.Button ok_button: wx.Button + download_immediately_checkbox: wx.CheckBox # or None + @staticmethod async def wait_for() -> NewGroupDialog: self = NewGroupDialog(ready=True) @@ -480,6 +482,13 @@ async def wait_for() -> NewGroupDialog: if self.ok_button is None: self.ok_button = add_group_dialog.FindWindow(id=wx.ID_SAVE) assert isinstance(self.ok_button, wx.Button) + + self.download_immediately_checkbox = add_group_dialog.FindWindow(name='cr-new-group-dialog__download-immediately-checkbox') + assert ( + self.download_immediately_checkbox is None or + isinstance(self.download_immediately_checkbox, wx.CheckBox) + ) + return self @staticmethod @@ -532,6 +541,12 @@ def _set_source(self, source_name: Optional[str]) -> None: self.source_field.SetSelection(selection_ci) source = property(_get_source, _set_source) + @property + def new_options_shown(self) -> bool: + return ( + self.download_immediately_checkbox is not None + ) + async def ok(self) -> None: click_button(self.ok_button) await wait_for(not_condition(window_condition('cr-new-group-dialog')), stacklevel_extra=1) From 83e8ce45259f726334d534b8189a8716900e8a09 Mon Sep 17 00:00:00 2001 From: David Foster Date: Sun, 25 Aug 2024 09:53:31 -0400 Subject: [PATCH 8/8] Update release notes --- RELEASE_NOTES.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 3a0b6a95..3fea465c 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -12,6 +12,11 @@ Release Notes ⋮ ### main +* Workflow improvements + * New URLs are downloaded immediately by default. + * Can immediately download the site for a newly created URL + with 1 extra click. + * Minor fixes * Fix closing a project to no longer have a couple of heap-use-after-free issues which could corrupt memory, potentially crashing Crystal later.