From 5bad273ed21205f40e57f1c0c4a10a46a309bfc7 Mon Sep 17 00:00:00 2001 From: David Foster Date: Fri, 5 Jan 2024 10:19:51 -0500 Subject: [PATCH 1/8] Entity Tree: Add lots of type annotations --- src/crystal/browser/__init__.py | 12 ++++++------ src/crystal/browser/entitytree.py | 31 ++++++++++++++++++------------- 2 files changed, 24 insertions(+), 19 deletions(-) diff --git a/src/crystal/browser/__init__.py b/src/crystal/browser/__init__.py index e19de533..00ffaf41 100644 --- a/src/crystal/browser/__init__.py +++ b/src/crystal/browser/__init__.py @@ -202,7 +202,7 @@ def _create_actions(self) -> None: self._forget_action = Action(wx.ID_ANY, '&Forget', wx.AcceleratorEntry(wx.ACCEL_CTRL, wx.WXK_BACK), - self._on_remove_entity, + self._on_forget_entity, enabled=False) self._download_action = Action(wx.ID_ANY, '&Download', @@ -465,7 +465,7 @@ def _on_add_group_dialog_ok(self, name: str, url_pattern: str, source): rg = ResourceGroup(self.project, name, url_pattern) rg.source = source - def _on_remove_entity(self, event): + def _on_forget_entity(self, event): self.entity_tree.selected_entity.delete() # TODO: This update() should happen in response to a delete # event fired by the entity itself. @@ -594,20 +594,20 @@ def start_server(self) -> 'ProjectServer': return self._project_server - def _on_selected_entity_changed(self, event): + def _on_selected_entity_changed(self, event: wx.TreeEvent) -> None: selected_entity = self.entity_tree.selected_entity # cache readonly = self._readonly # cache self._forget_action.enabled = ( (not readonly) and - type(selected_entity) in (ResourceGroup, RootResource)) + isinstance(selected_entity, (ResourceGroup, RootResource))) self._download_action.enabled = ( (not readonly) and selected_entity is not None) self._update_membership_action.enabled = ( (not readonly) and - type(selected_entity) is ResourceGroup) + isinstance(selected_entity, ResourceGroup)) self._view_action.enabled = ( - type(selected_entity) in (Resource, RootResource)) + isinstance(selected_entity, (Resource, RootResource))) # === Task Pane: Init === diff --git a/src/crystal/browser/entitytree.py b/src/crystal/browser/entitytree.py index 78d4f82c..8bb6e18f 100644 --- a/src/crystal/browser/entitytree.py +++ b/src/crystal/browser/entitytree.py @@ -525,11 +525,11 @@ def _entity_tooltip(self) -> str: # abstract raise NotImplementedError() @property - def entity(self): + def entity(self) -> NodeEntity: return self.resource @property - def _project(self): + def _project(self) -> Project: return self.resource.project # === Comparison === @@ -706,7 +706,7 @@ def __init__(self, root_resource: RootResource) -> None: def _entity_tooltip(self) -> str: return 'root URL' - def calculate_title(self): + def calculate_title(self) -> str: project = self.root_resource.project display_url = project.get_display_url(self.root_resource.url) if self.root_resource.name != '': @@ -715,7 +715,7 @@ def calculate_title(self): return '%s' % (display_url,) @property - def entity(self): + def entity(self) -> RootResource: return self.root_resource # === Comparison === @@ -738,12 +738,12 @@ def __init__(self, resource): def _entity_tooltip(self) -> str: return 'URL' - def calculate_title(self): + def calculate_title(self) -> str: project = self.resource.project return '%s' % project.get_display_url(self.resource.url) @property - def entity(self): + def entity(self) -> Resource: return self.resource # === Comparison === @@ -767,21 +767,22 @@ def __init__(self, resource, links): def _entity_tooltip(self) -> str: return 'URL' - def calculate_title(self): + def calculate_title(self) -> str: project = self.resource.project link_titles = ', '.join([self._full_title_of_link(link) for link in self.links]) return '%s - %s' % ( project.get_display_url(self.resource.url), link_titles) - def _full_title_of_link(self, link): + @staticmethod + def _full_title_of_link(link: Link) -> str: if link.title: return '%s: %s' % (link.type_title, link.title) else: return '%s' % link.type_title @property - def entity(self): + def entity(self) -> Resource: return self.resource # === Comparison === @@ -845,7 +846,7 @@ def __init__(self, resource_group: ResourceGroup) -> None: def icon_tooltip(self) -> Optional[str]: return 'Group' - def calculate_title(self): + def calculate_title(self) -> str: project = self.resource_group.project display_url = project.get_display_url(self.resource_group.url_pattern) if self.resource_group.name != '': @@ -854,7 +855,7 @@ def calculate_title(self): return '%s' % (display_url,) @property - def entity(self): + def entity(self) -> ResourceGroup: return self.resource_group # === Update === @@ -952,7 +953,11 @@ def __hash__(self) -> int: class GroupedLinkedResourcesNode(Node): - def __init__(self, resource_group, root_rsrc_nodes, linked_rsrc_nodes): + def __init__(self, + resource_group: ResourceGroup, + root_rsrc_nodes: List[RootResourceNode], + linked_rsrc_nodes: List[LinkedResourceNode], + ) -> None: self.resource_group = resource_group super().__init__() @@ -971,7 +976,7 @@ def __init__(self, resource_group, root_rsrc_nodes, linked_rsrc_nodes): def icon_tooltip(self) -> Optional[str]: return 'Grouped URLs' - def calculate_title(self): + def calculate_title(self) -> str: project = self.resource_group.project display_url_pattern = project.get_display_url(self.resource_group.url_pattern) if self.resource_group.name != '': From 9969f0171780b067e72b040103378c8383f858d3 Mon Sep 17 00:00:00 2001 From: David Foster Date: Thu, 4 Jan 2024 01:40:47 -0500 Subject: [PATCH 2/8] New Group: Fix suggested Source to be more reasonable (Take 1) --- src/crystal/browser/__init__.py | 22 ++++-------- src/crystal/browser/addgroup.py | 14 ++++---- src/crystal/browser/entitytree.py | 56 ++++++++++++++++++++----------- 3 files changed, 50 insertions(+), 42 deletions(-) diff --git a/src/crystal/browser/__init__.py b/src/crystal/browser/__init__.py index 00ffaf41..bb3586d7 100644 --- a/src/crystal/browser/__init__.py +++ b/src/crystal/browser/__init__.py @@ -349,7 +349,7 @@ def _create_button_bar(self, parent: wx.Window): # === Entity Pane: Properties === @property - def _selection_initial_url(self) -> Optional[str]: + def _suggested_url_or_url_pattern_for_selection(self) -> Optional[str]: selected_entity = self.entity_tree.selected_entity if isinstance(selected_entity, (Resource, RootResource)): return selected_entity.resource.url @@ -359,18 +359,8 @@ def _selection_initial_url(self) -> Optional[str]: return self.project.default_url_prefix @property - def _selection_initial_source(self) -> Optional[ResourceGroupSource]: - selected_entity = self.entity_tree.selected_entity - if isinstance(selected_entity, (Resource, RootResource)): - parent_of_selected_entity = self.entity_tree.parent_of_selected_entity - if isinstance(parent_of_selected_entity, (ResourceGroup, RootResource)): - return parent_of_selected_entity - else: - return None - elif isinstance(selected_entity, ResourceGroup): - return selected_entity.source - else: - return None + def _suggested_source_for_selection(self) -> Optional[ResourceGroupSource]: + return self.entity_tree.source_of_selected_entity # === Operations === @@ -436,7 +426,7 @@ def root_url_exists(url: str) -> bool: AddRootUrlDialog( self._frame, self._on_add_url_dialog_ok, url_exists_func=root_url_exists, - initial_url=self._selection_initial_url or '', + initial_url=self._suggested_url_or_url_pattern_for_selection or '', ) @fg_affinity @@ -453,8 +443,8 @@ def _on_add_group(self, event: wx.CommandEvent) -> None: AddGroupDialog( self._frame, self._on_add_group_dialog_ok, self.project, - initial_url=self._selection_initial_url or '', - initial_source=self._selection_initial_source) + initial_url_pattern=self._suggested_url_or_url_pattern_for_selection or '', + initial_source=self._suggested_source_for_selection) except CancelLoadUrls: pass diff --git a/src/crystal/browser/addgroup.py b/src/crystal/browser/addgroup.py index d16afee1..60570409 100644 --- a/src/crystal/browser/addgroup.py +++ b/src/crystal/browser/addgroup.py @@ -17,7 +17,7 @@ class AddGroupDialog: - _INITIAL_URL_WIDTH = AddRootUrlDialog._INITIAL_URL_WIDTH + _INITIAL_URL_PATTERN_WIDTH = AddRootUrlDialog._INITIAL_URL_WIDTH _MAX_VISIBLE_PREVIEW_URLS = 100 # === Init === @@ -26,7 +26,7 @@ def __init__(self, parent: wx.Window, on_finish: Callable[[str, str, ResourceGroupSource], None], project: Project, - initial_url: str='', + initial_url_pattern: str='', initial_source: Optional[ResourceGroupSource]=None, ) -> None: """ @@ -34,7 +34,7 @@ def __init__(self, * parent -- parent wx.Window that this dialog is attached to. * on_finish -- called when OK pressed on dialog. Is a callable(name, url_pattern, source). * project -- the project. - * initial_url -- overrides the initial URL displayed. + * initial_url_pattern -- overrides the initial URL pattern displayed. * initial_source -- overrides the initial source displayed. Raises: @@ -92,7 +92,7 @@ def __init__(self, content_sizer = wx.BoxSizer(wx.VERTICAL) content_sizer.Add( - self._create_fields(dialog, initial_url, initial_source), + self._create_fields(dialog, initial_url_pattern, initial_source), flag=wx.EXPAND) content_sizer.Add(preview_box, proportion=1, flag=wx.EXPAND|preview_box_flags, border=preview_box_border) @@ -114,7 +114,7 @@ def __init__(self, def _create_fields(self, parent: wx.Window, - initial_url: str, + initial_url_pattern: str, initial_source: Optional[ResourceGroupSource] ) -> wx.Sizer: fields_sizer = wx.FlexGridSizer(cols=2, @@ -123,8 +123,8 @@ def _create_fields(self, pattern_field_sizer = wx.BoxSizer(wx.VERTICAL) self.pattern_field = wx.TextCtrl( - parent, value=initial_url, - size=(self._INITIAL_URL_WIDTH, wx.DefaultCoord), + parent, value=initial_url_pattern, + size=(self._INITIAL_URL_PATTERN_WIDTH, wx.DefaultCoord), name='cr-add-group-dialog__pattern-field') bind(self.pattern_field, wx.EVT_TEXT, self._on_pattern_field_changed) self.pattern_field.Hint = 'https://example.com/post/*' diff --git a/src/crystal/browser/entitytree.py b/src/crystal/browser/entitytree.py index 8bb6e18f..50327aef 100644 --- a/src/crystal/browser/entitytree.py +++ b/src/crystal/browser/entitytree.py @@ -4,6 +4,7 @@ from crystal.doc.generic import Link from crystal.model import ( Project, ProjectHasTooManyRevisionsError, Resource, ResourceGroup, + ResourceGroupSource, ResourceRevision, RevisionBodyMissingError, RevisionDeletedError, RootResource, ) @@ -56,7 +57,7 @@ def __init__(self, self.root = RootNode(project, self.view.root, progress_listener) self._project = project self._group_nodes_need_updating = False - self._right_clicked_node = None + self._right_clicked_node = None # type: Optional[Node] project.listeners.append(self) @@ -80,26 +81,43 @@ def selected_entity(self) -> Optional[NodeEntity]: selected_node_view = self.view.selected_node if selected_node_view is None: return None - selected_node = selected_node_view.delegate - assert isinstance(selected_node, Node) - - return selected_node.entity + return self._node_for_node_view(selected_node_view).entity - # HACK: Violates the Law of Demeter rather substantially. @property - def parent_of_selected_entity(self): - selected_wxtreeitemid = self.view.peer.GetSelection() - if not selected_wxtreeitemid.IsOk(): + def source_of_selected_entity(self) -> Optional[ResourceGroupSource]: + selected_node_view = self.view.selected_node + if selected_node_view is None: return None + ancestor_node_view = self._parent_of_node_view(selected_node_view) + while ancestor_node_view is not None: + ancestor_node = self._node_for_node_view(ancestor_node_view) + if isinstance(ancestor_node, LinkedResourceNode): + return None + elif isinstance(ancestor_node, (RootResourceNode, ResourceGroupNode)): + return ancestor_node.entity + else: + # Continue + pass + + ancestor_node_view = self._parent_of_node_view(ancestor_node_view) + return None + + @staticmethod + def _node_for_node_view(node_view: NodeView) -> Node: + node = node_view.delegate + assert isinstance(node, Node) + return node + + @staticmethod + def _parent_of_node_view(node_view: NodeView) -> Optional[NodeView]: + assert node_view.peer is not None + tree_peer = node_view.peer.tree_peer - parent_wxtreeitemid = self.view.peer.GetItemParent(selected_wxtreeitemid) - if not parent_wxtreeitemid.IsOk(): + parent_node_id = tree_peer.GetItemParent(node_view.peer.node_id) + if not parent_node_id.IsOk(): return None - - parent_node_view = self.view.peer.GetItemData(parent_wxtreeitemid) - parent_node = parent_node_view.delegate - - return parent_node.entity + parent_node_view = tree_peer.GetItemData(parent_node_id) # type: NodeView + return parent_node_view # === Updates === @@ -156,8 +174,8 @@ def min_fetch_date_did_change(self) -> None: # === Event: Right Click === - def on_right_click(self, event, node_view): - node = node_view.delegate + def on_right_click(self, event, node_view: NodeView) -> None: + node = self._node_for_node_view(node_view) self._right_clicked_node = node # Create popup menu @@ -231,7 +249,7 @@ def _on_get_tooltip_event(self, event: wx.Event) -> None: def _icon_tooltip_for_tree_item_id(self, tree_item_id) -> Optional[str]: node_view = self.peer.GetItemData(tree_item_id) # type: NodeView - node = cast(Node, node_view.delegate) + node = self._node_for_node_view(node_view) return node.icon_tooltip # === Dispose === From cf92f9c00f5952157a5a7c61aa5b79b92d94d30d Mon Sep 17 00:00:00 2001 From: David Foster Date: Thu, 4 Jan 2024 02:05:18 -0500 Subject: [PATCH 3/8] New Root URL, New Group: Fix suggested Name to be more reasonable --- src/crystal/browser/__init__.py | 8 +++++++- src/crystal/browser/addgroup.py | 9 ++++++--- src/crystal/browser/addrooturl.py | 7 ++++--- src/crystal/browser/entitytree.py | 23 +++++++++++++++++++++++ src/crystal/doc/html/soup.py | 5 ++++- src/crystal/tests/test_workflows.py | 3 ++- 6 files changed, 46 insertions(+), 9 deletions(-) diff --git a/src/crystal/browser/__init__.py b/src/crystal/browser/__init__.py index bb3586d7..183012ee 100644 --- a/src/crystal/browser/__init__.py +++ b/src/crystal/browser/__init__.py @@ -362,6 +362,10 @@ def _suggested_url_or_url_pattern_for_selection(self) -> Optional[str]: def _suggested_source_for_selection(self) -> Optional[ResourceGroupSource]: return self.entity_tree.source_of_selected_entity + @property + def _suggested_name_for_selection(self) -> Optional[str]: + return self.entity_tree.name_of_selected_entity + # === Operations === def close(self) -> None: @@ -427,6 +431,7 @@ def root_url_exists(url: str) -> bool: self._frame, self._on_add_url_dialog_ok, url_exists_func=root_url_exists, initial_url=self._suggested_url_or_url_pattern_for_selection or '', + initial_name=self._suggested_name_for_selection or '', ) @fg_affinity @@ -444,7 +449,8 @@ def _on_add_group(self, event: wx.CommandEvent) -> None: self._frame, self._on_add_group_dialog_ok, self.project, initial_url_pattern=self._suggested_url_or_url_pattern_for_selection or '', - initial_source=self._suggested_source_for_selection) + initial_source=self._suggested_source_for_selection, + initial_name=self._suggested_name_for_selection or '') except CancelLoadUrls: pass diff --git a/src/crystal/browser/addgroup.py b/src/crystal/browser/addgroup.py index 60570409..e55ebbb8 100644 --- a/src/crystal/browser/addgroup.py +++ b/src/crystal/browser/addgroup.py @@ -28,6 +28,7 @@ def __init__(self, project: Project, initial_url_pattern: str='', initial_source: Optional[ResourceGroupSource]=None, + initial_name: str='', ) -> None: """ Arguments: @@ -36,6 +37,7 @@ def __init__(self, * project -- the project. * initial_url_pattern -- overrides the initial URL pattern displayed. * initial_source -- overrides the initial source displayed. + * initial_name -- overrides the initial name displayed. Raises: * CancelLoadUrls @@ -92,7 +94,7 @@ def __init__(self, content_sizer = wx.BoxSizer(wx.VERTICAL) content_sizer.Add( - self._create_fields(dialog, initial_url_pattern, initial_source), + self._create_fields(dialog, initial_url_pattern, initial_source, initial_name), flag=wx.EXPAND) content_sizer.Add(preview_box, proportion=1, flag=wx.EXPAND|preview_box_flags, border=preview_box_border) @@ -115,7 +117,8 @@ def __init__(self, def _create_fields(self, parent: wx.Window, initial_url_pattern: str, - initial_source: Optional[ResourceGroupSource] + initial_source: Optional[ResourceGroupSource], + initial_name: str, ) -> wx.Sizer: fields_sizer = wx.FlexGridSizer(cols=2, vgap=_FORM_ROW_SPACING, hgap=_FORM_LABEL_INPUT_SPACING) @@ -157,7 +160,7 @@ def _create_fields(self, fields_sizer.Add(wx.StaticText(parent, label='Name:', style=wx.ALIGN_RIGHT), flag=wx.EXPAND) self.name_field = wx.TextCtrl( - parent, + parent, value=initial_name, name='cr-add-group-dialog__name-field') self.name_field.Hint = 'Post' self.name_field.SetSelection(-1, -1) # select all upon focus diff --git a/src/crystal/browser/addrooturl.py b/src/crystal/browser/addrooturl.py index d33e2ea1..58732329 100644 --- a/src/crystal/browser/addrooturl.py +++ b/src/crystal/browser/addrooturl.py @@ -27,6 +27,7 @@ def __init__(self, on_finish: Callable[[str, str], None], url_exists_func: Callable[[str], bool], initial_url: str='', + initial_name: str='', ) -> None: """ Arguments: @@ -52,7 +53,7 @@ def __init__(self, bind(dialog, wx.EVT_CLOSE, self._on_close) bind(dialog, wx.EVT_WINDOW_DESTROY, self._on_destroyed) - dialog_sizer.Add(self._create_fields(dialog, initial_url), flag=wx.EXPAND|wx.ALL, + dialog_sizer.Add(self._create_fields(dialog, initial_url, initial_name), flag=wx.EXPAND|wx.ALL, border=_WINDOW_INNER_PADDING) dialog_sizer.Add(dialog.CreateButtonSizer(wx.OK|wx.CANCEL), flag=wx.EXPAND|wx.BOTTOM, border=_WINDOW_INNER_PADDING) @@ -75,7 +76,7 @@ def __init__(self, if os.environ.get('CRYSTAL_RUNNING_TESTS', 'False') == 'True': AddRootUrlDialog._last_opened = self - def _create_fields(self, parent: wx.Window, initial_url: str) -> wx.Sizer: + def _create_fields(self, parent: wx.Window, initial_url: str, initial_name: str) -> wx.Sizer: fields_sizer = wx.FlexGridSizer(rows=2, cols=2, vgap=_FORM_ROW_SPACING, hgap=_FORM_LABEL_INPUT_SPACING) fields_sizer.AddGrowableCol(1) @@ -115,7 +116,7 @@ def _create_fields(self, parent: wx.Window, initial_url: str) -> wx.Sizer: name_field_and_space = wx.BoxSizer(wx.HORIZONTAL) if True: self.name_field = wx.TextCtrl( - parent, + parent, value=initial_name, name='cr-add-url-dialog__name-field') self.name_field.Hint = 'Home' self.name_field.SetSelection(-1, -1) # select all upon focus diff --git a/src/crystal/browser/entitytree.py b/src/crystal/browser/entitytree.py index 50327aef..5c5f15cc 100644 --- a/src/crystal/browser/entitytree.py +++ b/src/crystal/browser/entitytree.py @@ -2,6 +2,7 @@ from crystal.browser.icons import BADGED_TREE_NODE_ICON, TREE_NODE_ICONS from crystal.doc.generic import Link +from crystal.doc.html.soup import TEXT_LINK_TYPE_TITLE from crystal.model import ( Project, ProjectHasTooManyRevisionsError, Resource, ResourceGroup, ResourceGroupSource, @@ -102,6 +103,28 @@ def source_of_selected_entity(self) -> Optional[ResourceGroupSource]: ancestor_node_view = self._parent_of_node_view(ancestor_node_view) return None + @property + def name_of_selected_entity(self) -> Optional[str]: + entity = self.selected_entity + if isinstance(entity, (RootResource, ResourceGroup)): + return entity.name + elif isinstance(entity, Resource): + selected_node_view = self.view.selected_node + if selected_node_view is None: + return None + selected_node = self._node_for_node_view(selected_node_view) + if isinstance(selected_node, LinkedResourceNode): + for link in selected_node.links: + if (link.type_title == TEXT_LINK_TYPE_TITLE and + link.title is not None and + link.title != ''): + return link.title + return None + else: + return None + else: + return None + @staticmethod def _node_for_node_view(node_view: NodeView) -> Node: node = node_view.delegate diff --git a/src/crystal/doc/html/soup.py b/src/crystal/doc/html/soup.py index fb1cd605..6b40a990 100644 --- a/src/crystal/doc/html/soup.py +++ b/src/crystal/doc/html/soup.py @@ -57,6 +57,9 @@ class _XPaths: ) for T in _PARSER_LIBRARY_T_CHOICES} +TEXT_LINK_TYPE_TITLE = 'Link' + + def parse_html_and_links( html_bytes: bytes, declared_charset: Optional[str], @@ -119,7 +122,7 @@ def parse_html_and_links( embedded = False if tag_name == 'a': title = html.tag_string(tag) - type_title = 'Link' + type_title = TEXT_LINK_TYPE_TITLE # 'Link' elif tag_name == 'link' and ( ('rel' in tag_attrs and 'stylesheet' in tag_attrs['rel']) or ( 'type' in tag_attrs and tag_attrs['type'] == 'text/css') or ( diff --git a/src/crystal/tests/test_workflows.py b/src/crystal/tests/test_workflows.py index a0929bed..9c2be20a 100644 --- a/src/crystal/tests/test_workflows.py +++ b/src/crystal/tests/test_workflows.py @@ -126,7 +126,7 @@ async def test_can_download_and_serve_a_static_site() -> None: click_button(mw.add_group_button) agd = await AddGroupDialog.wait_for() - assert '' == agd.name_field.Value # default name = (nothing) + assert '|<' == agd.name_field.Value # default name = (from first text link) assert comic1_url == agd.pattern_field.Value # default pattern = (from resource) assert 'Home' == agd.source # default source = (from resource parent) assert agd.name_field.HasFocus # default focused field @@ -1078,6 +1078,7 @@ async def test_can_download_a_static_site_with_unnamed_root_urls_and_groups() -> agd.pattern_field.Value = comic_pattern agd.source = home_url + agd.name_field.Value = '' await agd.ok() # 1. Ensure the new resource group does now group sub-resources From de82dc493b80e0acf6a3281e1a0d755e3d8a07a5 Mon Sep 17 00:00:00 2001 From: David Foster Date: Fri, 5 Jan 2024 10:20:11 -0500 Subject: [PATCH 4/8] Entity Tree: Intelligently retarget selection when new root URL or group created for selected link --- src/crystal/browser/__init__.py | 24 ++++--- src/crystal/browser/entitytree.py | 109 ++++++++++++++++++++++-------- src/crystal/ui/tree.py | 91 ++++++++++++++++++++++--- 3 files changed, 179 insertions(+), 45 deletions(-) diff --git a/src/crystal/browser/__init__.py b/src/crystal/browser/__init__.py index 183012ee..b5f4140f 100644 --- a/src/crystal/browser/__init__.py +++ b/src/crystal/browser/__init__.py @@ -350,11 +350,13 @@ def _create_button_bar(self, parent: wx.Window): @property def _suggested_url_or_url_pattern_for_selection(self) -> Optional[str]: - selected_entity = self.entity_tree.selected_entity - if isinstance(selected_entity, (Resource, RootResource)): - return selected_entity.resource.url - elif isinstance(selected_entity, ResourceGroup): - return selected_entity.url_pattern + selected_entity_pair = self.entity_tree.selected_entity_pair + selected_or_related_entity = selected_entity_pair[0] or selected_entity_pair[1] + + if isinstance(selected_or_related_entity, (Resource, RootResource)): + return selected_or_related_entity.resource.url + elif isinstance(selected_or_related_entity, ResourceGroup): + return selected_or_related_entity.url_pattern else: return self.project.default_url_prefix @@ -462,7 +464,10 @@ def _on_add_group_dialog_ok(self, name: str, url_pattern: str, source): rg.source = source def _on_forget_entity(self, event): - self.entity_tree.selected_entity.delete() + selected_entity_pair = self.entity_tree.selected_entity_pair + selected_or_related_entity = selected_entity_pair[0] or selected_entity_pair[1] + + selected_or_related_entity.delete() # TODO: This update() should happen in response to a delete # event fired by the entity itself. self.entity_tree.update() @@ -591,11 +596,14 @@ def start_server(self) -> 'ProjectServer': return self._project_server def _on_selected_entity_changed(self, event: wx.TreeEvent) -> None: - selected_entity = self.entity_tree.selected_entity # cache + selected_entity_pair = self.entity_tree.selected_entity_pair # cache + selected_entity = selected_entity_pair[0] + selected_or_related_entity = selected_entity_pair[0] or selected_entity_pair[1] + readonly = self._readonly # cache self._forget_action.enabled = ( (not readonly) and - isinstance(selected_entity, (ResourceGroup, RootResource))) + isinstance(selected_or_related_entity, (ResourceGroup, RootResource))) self._download_action.enabled = ( (not readonly) and selected_entity is not None) diff --git a/src/crystal/browser/entitytree.py b/src/crystal/browser/entitytree.py index 5c5f15cc..9af323bb 100644 --- a/src/crystal/browser/entitytree.py +++ b/src/crystal/browser/entitytree.py @@ -79,19 +79,27 @@ def peer(self) -> wx.TreeCtrl: @property def selected_entity(self) -> Optional[NodeEntity]: + return self.selected_entity_pair[0] + + @property + def selected_entity_pair(self) -> Tuple[Optional[NodeEntity], Optional[NodeEntity]]: selected_node_view = self.view.selected_node if selected_node_view is None: - return None - return self._node_for_node_view(selected_node_view).entity + return (None, None) + selected_node = Node.for_node_view(selected_node_view) + return ( + selected_node.entity, + selected_node.related_entity, + ) @property def source_of_selected_entity(self) -> Optional[ResourceGroupSource]: selected_node_view = self.view.selected_node if selected_node_view is None: return None - ancestor_node_view = self._parent_of_node_view(selected_node_view) + ancestor_node_view = selected_node_view.parent while ancestor_node_view is not None: - ancestor_node = self._node_for_node_view(ancestor_node_view) + ancestor_node = Node.for_node_view(ancestor_node_view) if isinstance(ancestor_node, LinkedResourceNode): return None elif isinstance(ancestor_node, (RootResourceNode, ResourceGroupNode)): @@ -100,7 +108,7 @@ def source_of_selected_entity(self) -> Optional[ResourceGroupSource]: # Continue pass - ancestor_node_view = self._parent_of_node_view(ancestor_node_view) + ancestor_node_view = ancestor_node_view.parent return None @property @@ -112,7 +120,7 @@ def name_of_selected_entity(self) -> Optional[str]: selected_node_view = self.view.selected_node if selected_node_view is None: return None - selected_node = self._node_for_node_view(selected_node_view) + selected_node = Node.for_node_view(selected_node_view) if isinstance(selected_node, LinkedResourceNode): for link in selected_node.links: if (link.type_title == TEXT_LINK_TYPE_TITLE and @@ -125,23 +133,6 @@ def name_of_selected_entity(self) -> Optional[str]: else: return None - @staticmethod - def _node_for_node_view(node_view: NodeView) -> Node: - node = node_view.delegate - assert isinstance(node, Node) - return node - - @staticmethod - def _parent_of_node_view(node_view: NodeView) -> Optional[NodeView]: - assert node_view.peer is not None - tree_peer = node_view.peer.tree_peer - - parent_node_id = tree_peer.GetItemParent(node_view.peer.node_id) - if not parent_node_id.IsOk(): - return None - parent_node_view = tree_peer.GetItemData(parent_node_id) # type: NodeView - return parent_node_view - # === Updates === def update(self): @@ -198,7 +189,7 @@ def min_fetch_date_did_change(self) -> None: # === Event: Right Click === def on_right_click(self, event, node_view: NodeView) -> None: - node = self._node_for_node_view(node_view) + node = Node.for_node_view(node_view) self._right_clicked_node = node # Create popup menu @@ -270,9 +261,9 @@ def _on_mouse_motion(self, event: wx.MouseEvent) -> None: def _on_get_tooltip_event(self, event: wx.Event) -> None: event.tooltip_cell[0] = self._icon_tooltip_for_tree_item_id(event.tree_item_id) - def _icon_tooltip_for_tree_item_id(self, tree_item_id) -> Optional[str]: + def _icon_tooltip_for_tree_item_id(self, tree_item_id: wx.TreeItemId) -> Optional[str]: node_view = self.peer.GetItemData(tree_item_id) # type: NodeView - node = self._node_for_node_view(node_view) + node = Node.for_node_view(node_view) return node.icon_tooltip # === Dispose === @@ -300,6 +291,12 @@ class Node: def __init__(self) -> None: self._children = [] # type: List[Node] + @staticmethod + def for_node_view(node_view: NodeView) -> Node: + node = node_view.delegate + assert isinstance(node, Node) + return node + # === Properties === def _get_view(self) -> NodeView: @@ -322,6 +319,8 @@ def set_children(self, Raises: * CancelOpenProject """ + # NOTE: Very important. Needed to reuse most nodes when changing + # children list to a similar children list. value = _sequence_with_matching_elements_replaced(value, self._children) self._children = value self.view.set_children([child.view for child in value], progress_listener) @@ -340,6 +339,13 @@ def entity(self) -> Optional[NodeEntity]: """ return None + @property + def related_entity(self) -> Optional[NodeEntity]: + """ + The entity related to this node, or None if not applicable. + """ + return None + # === Updates === def update_descendants(self) -> None: @@ -636,7 +642,7 @@ def bg_task() -> None: # === Updates === - def update_children(self): + def update_children(self) -> None: """ Updates this node's children. Should be called whenever project entities change or the underlying resource's links change. @@ -731,6 +737,44 @@ def update_children(self): ), 'Embedded URLs')) self.children = children + + def on_selection_deleted(self, + old_selected_node_view: NodeView, + ) -> Optional[NodeView]: + old_selected_node = Node.for_node_view(old_selected_node_view) + if isinstance(old_selected_node, _ResourceNode): + # Probably this node still exists but in a different location. + # Try to find that location. + for child in self.children: + if old_selected_node == child: + return child.view + for grandchild in child.children: + if old_selected_node == grandchild: + return grandchild.view + + # Maybe this node changed type, either: + # 1. {LinkedResourceNode, NormalResourceNode} -> RootResourceNode + # 2. RootResourceNode -> {LinkedResourceNode, NormalResourceNode} + # Try to find a node with the same URL even if different type. + for child in self.children: + if isinstance(child, _ResourceNode): + if old_selected_node.resource == child.resource: + return child.view + for grandchild in child.children: + if isinstance(grandchild, _ResourceNode): + if old_selected_node.resource == grandchild.resource: + return grandchild.view + + return None + elif isinstance(old_selected_node, GroupedLinkedResourcesNode): + # Probably the first child still exists but in a different location. + # Try to find that location. + children = old_selected_node.children + if len(children) == 0: + return None + return self.on_selection_deleted(children[0].view) + else: + return None class RootResourceNode(_ResourceNode): @@ -1031,8 +1075,17 @@ def calculate_title(self) -> str: len(self.children), '' if len(self.children) == 1 else 's') + #override + @property + def entity(self) -> Optional[NodeEntity]: + # This node does NOT represent a ResourceGroup despite being related to one. + return None + + #override @property - def entity(self): + def related_entity(self) -> Optional[NodeEntity]: + # This node groups together various _ResourceNode entities that + # are in the same ResourceGroup return self.resource_group # === Comparison === diff --git a/src/crystal/ui/tree.py b/src/crystal/ui/tree.py index 28bc05d4..81bd2757 100644 --- a/src/crystal/ui/tree.py +++ b/src/crystal/ui/tree.py @@ -18,7 +18,7 @@ wrapped_object_deleted_error_raising ) from crystal.util.xthreading import fg_affinity -from typing import Callable, Dict, List, NewType, NoReturn, Optional, Tuple, Union +from typing import Callable, Container, Dict, List, NewType, NoReturn, Optional, Tuple, Union import wx @@ -96,6 +96,8 @@ def __init__(self, parent_peer: wx.Window, *, name: Optional[str]=None) -> None: for event_type in _EVENT_TYPE_2_DELEGATE_CALLABLE_ATTR: bind(self.peer, event_type, self._dispatch_event, self.peer) + # === Properties === + def _get_root(self) -> NodeView: return self._root def _set_root(self, value: NodeView) -> None: @@ -103,16 +105,24 @@ def _set_root(self, value: NodeView) -> None: self._root._attach(self._root_peer) root = property(_get_root, _set_root) + # TODO: Rename: selection @property def selected_node(self) -> Optional[NodeView]: + return self.selected_node_in(self.peer) + + # TODO: Rename: selection_in + @staticmethod + def selected_node_in(tree_peer: wx.TreeCtrl) -> Optional[NodeView]: try: - selected_node_id = self.peer.GetSelection() + selected_node_id = tree_peer.GetSelection() except Exception as e: if is_wrapped_object_deleted_error(e): return None else: raise - return self.peer.GetItemData(selected_node_id) if selected_node_id.IsOk() else None + if not selected_node_id.IsOk(): + return None + return tree_peer.GetItemData(selected_node_id) def get_image_id_for_bitmap(self, bitmap: wx.Bitmap) -> ImageIndex: """ @@ -126,11 +136,13 @@ def get_image_id_for_bitmap(self, bitmap: wx.Bitmap) -> ImageIndex: self._bitmap_2_image_id[bitmap] = ImageIndex(image_id) return image_id + # === Operations === + def expand(self, node_view): self.peer.Expand(node_view.peer.node_id) # Notified when any interesting event occurs on the peer - def _dispatch_event(self, event) -> None: + def _dispatch_event(self, event: wx.TreeEvent) -> None: node_id = event.GetItem() node_view = self.peer.GetItemData(node_id) # type: NodeView @@ -150,7 +162,7 @@ def dispose(self) -> None: class _OrderedTreeCtrl(wx.TreeCtrl): - def OnCompareItems(self, item1, item2): + def OnCompareItems(self, item1: wx.TreeItemId, item2: wx.TreeItemId) -> int: (item1_view, item2_view) = (self.GetItemData(item1), self.GetItemData(item2)) assert isinstance(item1_view, NodeView) and isinstance(item2_view, NodeView) (order_index_1, order_index_2) = ( @@ -200,6 +212,8 @@ def __init__(self) -> None: self._icon_set = None # type: Optional[IconSet] self._children = [] # type: List[NodeView] + # === Properties === + def _get_title(self) -> str: return self._title def _set_title(self, value: str) -> None: @@ -286,13 +300,28 @@ def set_children(self, child._attach(NodeViewPeer(self.peer._tree, self.peer.AppendItem(''))) else: # Replace existing children, preserving old ones that match new ones + old_children_set = set(old_children) - children_to_delete = old_children_set - set(new_children) - for child in children_to_delete: - if child.peer is not None: - child.peer.Delete() + # 1. Delete some children + # 2. Capture selected node if it is in the deleted region + if True: + children_to_delete = old_children_set - set(new_children) + + on_selection_deleted = getattr( + self.delegate, 'on_selection_deleted', None + ) # type: Optional[Callable[[NodeView], Optional[NodeView]]] + old_selection_to_retarget = None # type: Optional[NodeView] + if on_selection_deleted is not None: + old_selection = TreeView.selected_node_in(self.peer.tree_peer) + if old_selection is not None and old_selection.is_descendent_of_any(children_to_delete): + old_selection_to_retarget = old_selection # capture + + for child in children_to_delete: + if child.peer is not None: + child.peer.Delete() + # Add some children children_to_add = [new_child for new_child in new_children if new_child not in old_children_set] for child in children_to_add: child._attach(NodeViewPeer(self.peer._tree, self.peer.AppendItem(''))) @@ -301,6 +330,17 @@ def set_children(self, for (index, child) in enumerate(new_children): child._order_index = index # type: ignore[attr-defined] self.peer.SortChildren() + + # If old selected node was in the deleted region, try to + # intelligently retarget the selection to something else + if on_selection_deleted is not None and old_selection_to_retarget is not None: + new_selection = on_selection_deleted(old_selection_to_retarget) + if new_selection is None: + self.peer.tree_peer.Unselect() + else: + assert new_selection.peer is not None + new_selection.peer.SelectItem() + except WindowDeletedError: pass @@ -314,12 +354,31 @@ def append_child(self, child: NodeView) -> None: except WindowDeletedError: pass + @property + def parent(self) -> Optional[NodeView]: + if not self.peer: + raise ValueError('Cannot lookup parent when not attached to a tree.') + parent_treeitemid = self.peer.GetItemParent() + if not parent_treeitemid.IsOk(): + return None + return self.peer.tree_peer.GetItemData(parent_treeitemid) + + def is_descendent_of_any(self, ancestor_node_views: Container[NodeView]) -> bool: + cur_node_view = self # type: Optional[NodeView] + while cur_node_view is not None: + if cur_node_view in ancestor_node_views: + return True + cur_node_view = cur_node_view.parent + return False + @property def _tree(self) -> TreeView: if not self.peer: raise ValueError('Not attached to a tree.') return self.peer._tree + # === Operations === + def _attach(self, peer: NodeViewPeer) -> None: old_peer = self.peer # capture if old_peer: @@ -357,6 +416,10 @@ def dispose(self) -> None: class NodeViewPeer(tuple): + """ + Thin wrapper around a wxPython tree item that makes the underlying API safer to use. + """ + def __new__(cls, tree: TreeView, node_id: wx.TreeItemId): return tuple.__new__(cls, (tree, node_id)) @@ -395,6 +458,7 @@ def SetItemHasChildren(self, has: bool) -> None: with wrapped_object_deleted_error_ignored(): self.tree_peer.SetItemHasChildren(node_id, has) + # TODO: Delete unused method @fg_affinity def GetFirstChild(self) -> Tuple[wx.TreeItemId, object]: node_id = self.node_id # cache @@ -404,6 +468,15 @@ def GetFirstChild(self) -> Tuple[wx.TreeItemId, object]: else: self._raise_no_longer_exists() + @fg_affinity + def GetItemParent(self) -> wx.TreeItemId: + node_id = self.node_id # cache + if node_id.IsOk(): + with wrapped_object_deleted_error_raising(self._raise_no_longer_exists): + return self.tree_peer.GetItemParent(node_id) + else: + self._raise_no_longer_exists() + @fg_affinity def AppendItem(self, text: str, *args) -> wx.TreeItemId: node_id = self.node_id # cache From f6301bebbf71ab622b0b8cd9d26bd89e509ec8aa Mon Sep 17 00:00:00 2001 From: David Foster Date: Thu, 4 Jan 2024 09:01:02 -0500 Subject: [PATCH 5/8] New Root URL, New Group: Add test coverage --- src/crystal/tests/index.py | 2 + src/crystal/tests/test_addgroup.py | 245 +++++++++++++++++++++++++++ src/crystal/tests/test_addrooturl.py | 199 +++++++++++++++++++++- src/crystal/tests/test_workflows.py | 14 +- src/crystal/tests/util/controls.py | 12 +- src/crystal/tests/util/windows.py | 17 +- 6 files changed, 474 insertions(+), 15 deletions(-) create mode 100644 src/crystal/tests/test_addgroup.py diff --git a/src/crystal/tests/index.py b/src/crystal/tests/index.py index 9722dba4..3ae4d7b9 100644 --- a/src/crystal/tests/index.py +++ b/src/crystal/tests/index.py @@ -1,4 +1,5 @@ from crystal.tests import ( + test_addgroup, test_addrooturl, test_disk_io_errors, test_download, @@ -51,6 +52,7 @@ def _test_functions_in_module(mod) -> List[Callable]: # TODO: Avoid the need to manually enumerate all test modules individually _TEST_FUNCS = ( + _test_functions_in_module(test_addgroup) + _test_functions_in_module(test_addrooturl) + _test_functions_in_module(test_disk_io_errors) + _test_functions_in_module(test_download) + diff --git a/src/crystal/tests/test_addgroup.py b/src/crystal/tests/test_addgroup.py new file mode 100644 index 00000000..67c3f41b --- /dev/null +++ b/src/crystal/tests/test_addgroup.py @@ -0,0 +1,245 @@ +from crystal.tests.util.controls import click_button, TreeItem +from crystal.tests.util.server import served_project +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, + wait_for, +) +from crystal.tests.util.windows import ( + AddGroupDialog, AddUrlDialog, EntityTree, OpenOrCreateDialog, +) +from crystal.util.xos import is_windows +import re +from unittest import skip + + +# === Test: Create & Delete Standalone === + +async def test_can_create_group_with_source(*, with_source: bool=True) -> None: + with served_project('testdata_xkcd.crystalproj.zip') as sp: + # Define URLs + home_url = sp.get_request_url('https://xkcd.com/') + comic_pattern = sp.get_request_url('https://xkcd.com/#/') + + async with (await OpenOrCreateDialog.wait_for()).create() as (mw, _): + root_ti = TreeItem.GetRootItem(mw.entity_tree.window) + assert root_ti is not None + () = root_ti.Children + + # If will need a source later, create one now + if with_source: + assert mw.add_url_button.Enabled + click_button(mw.add_url_button) + aud = await AddUrlDialog.wait_for() + + aud.name_field.Value = 'Home' + aud.url_field.Value = home_url + await aud.ok() + + # Create a group + if True: + # Ensure nothing selected + if not is_windows(): + selected_ti = TreeItem.GetSelection(mw.entity_tree.window) + assert (selected_ti is None) or (selected_ti == root_ti) + + assert mw.add_group_button.Enabled + click_button(mw.add_group_button) + agd = await AddGroupDialog.wait_for() + + # Ensure prepopulates reasonable information + if not is_windows(): + assert '' == agd.pattern_field.Value + assert '' == agd.name_field.Value + assert None == agd.source + else: + # Windows appears to have some kind of race condition that + # sometimes causes the Home tree item to be selected initially + assert agd.pattern_field.Value in ['', home_url] + assert agd.name_field.Value in ['', 'Home'] + assert None == agd.source + assert agd.pattern_field.HasFocus # default focused field + + # Input new URL pattern with wildcard, to match comics + agd.pattern_field.Value = comic_pattern + + # Ensure preview members show the new matching URLs (i.e. none) + member_urls = [ + agd.preview_members_list.GetString(i) + for i in range(agd.preview_members_list.GetCount()) + ] + assert [] == member_urls # no comics discovered yet + + if with_source: + agd.source = 'Home' + agd.name_field.Value = 'Comic' + await agd.ok() + + # Ensure appearance is correct + (comic_ti,) = [ + child for child in root_ti.Children + if child.Text.startswith(f'{comic_pattern} - ') + ] + assert f'{comic_pattern} - Comic' == comic_ti.Text + await _assert_tree_item_icon_tooltip_contains(comic_ti, 'Group') + + # Currently, an entirely new group is NOT selected automatically. + # This behavior might be changed in the future. + if not is_windows(): + selected_ti = TreeItem.GetSelection(mw.entity_tree.window) + assert (selected_ti is None) or (selected_ti == root_ti) + + # Forget group + if True: + comic_ti.SelectItem() + assert mw.forget_button.IsEnabled() + click_button(mw.forget_button) + + # Ensure cannot find group + () = [ + child for child in root_ti.Children + if child.Text.startswith(f'{comic_pattern} - ') + ] + if not is_windows(): + selected_ti = TreeItem.GetSelection(mw.entity_tree.window) + assert (selected_ti is None) or (selected_ti == root_ti) + + +async def test_can_create_group_with_no_source() -> None: + await test_can_create_group_with_source(with_source=False) + + +@skip('covered by: test_can_create_group_with_source') +async def test_can_forget_group() -> None: + pass + + +@skip('not yet automated') +async def test_when_forget_group_then_related_root_urls_and_revisions_are_not_deleted() -> None: + pass + + +# === Test: Create & Delete from Links === + +async def test_given_resource_node_with_multiple_link_children_matching_url_pattern_can_create_new_group_to_bundle_those_links_together() -> None: + with served_project('testdata_xkcd.crystalproj.zip') as sp: + # Define URLs + if True: + home_url = sp.get_request_url('https://xkcd.com/') + + comic1_url = sp.get_request_url('https://xkcd.com/1/') + comic2_url = sp.get_request_url('https://xkcd.com/2/') + comic_pattern = sp.get_request_url('https://xkcd.com/#/') + + async with (await OpenOrCreateDialog.wait_for()).create() as (mw, _): + # Create home URL + if True: + root_ti = TreeItem.GetRootItem(mw.entity_tree.window) + assert root_ti is not None + () = root_ti.Children + + assert mw.add_url_button.Enabled + click_button(mw.add_url_button) + aud = await AddUrlDialog.wait_for() + + aud.name_field.Value = 'Home' + aud.url_field.Value = home_url + await aud.ok() + (home_ti,) = root_ti.Children + + # Expand home URL + home_ti.Expand() + await wait_for_download_to_start_and_finish(mw.task_tree) + assert first_child_of_tree_item_is_not_loading_condition(home_ti)() + + # Select a comic link from the home URL + (comic1_ti,) = [ + child for child in home_ti.Children + if child.Text.startswith(f'{comic1_url} - ') + ] # ensure did find sub-resource for Comic #1 + assert f'{comic1_url} - Link: |<, Link: |<' == comic1_ti.Text # ensure expected links grouped + comic1_ti.SelectItem() + + # Create a group to bundle the comic links together + if True: + assert mw.add_group_button.Enabled + click_button(mw.add_group_button) + agd = await AddGroupDialog.wait_for() + + # Ensure prepopulates reasonable information + assert comic1_url == agd.pattern_field.Value # default pattern = (from resource) + assert '|<' == agd.name_field.Value # default name = (from first text link) + assert 'Home' == agd.source # default source = (from resource parent) + assert agd.pattern_field.HasFocus # default focused field + + # Ensure preview members show the 1 URL + assert ( + agd.preview_members_pane is None or # always expanded + agd.preview_members_pane.IsExpanded() # expanded by default + ) + member_urls = [ + agd.preview_members_list.GetString(i) + for i in range(agd.preview_members_list.GetCount()) + ] + assert [comic1_url] == member_urls # contains exact match of pattern + + # Input new URL pattern with wildcard, to match other comics + agd.pattern_field.Value = comic_pattern + + # Ensure preview members show the new matching URLs + member_urls = [ + agd.preview_members_list.GetString(i) + for i in range(agd.preview_members_list.GetCount()) + ] + assert comic1_url in member_urls # contains first comic + assert len(member_urls) >= 2 # contains last comic too + + # Input new name + agd.name_field.Value = 'Comic' + + await agd.ok() + + # 1. Ensure the new resource group does now bundle the comic links together + # 2. Ensure the bundled link is selected immediately after closing the add group dialog + if True: + (grouped_subresources_ti,) = [ + child for child in home_ti.Children + if child.Text.startswith(f'{comic_pattern} - ') + ] # ensure did find grouped sub-resources + assert re.fullmatch( + rf'{re.escape(comic_pattern)} - \d+ of Comic', # title format of grouped sub-resources + grouped_subresources_ti.Text) + + # Ensure the bundled link is selected immediately after closing the add group dialog + assert grouped_subresources_ti.IsExpanded() + (comic1_ti,) = [ + child for child in grouped_subresources_ti.Children + if child.Text.startswith(f'{comic1_url} - ') + ] # contains first comic + assert len(grouped_subresources_ti.Children) >= 2 # contains last comic too + assert comic1_ti.IsSelected() + + # Forget the group to unbundle the links + if True: + grouped_subresources_ti.SelectItem() + assert mw.forget_button.IsEnabled() + click_button(mw.forget_button) + + # 1. Ensure can find the first unbundled link + # 2. Ensure that first unbundled link is selected immediately after forgetting the group + (comic1_ti,) = [ + child for child in home_ti.Children + if child.Text.startswith(f'{comic1_url} - ') + ] # ensure did find sub-resource for Comic #1 + assert comic1_ti.IsSelected() + + +@skip('covered by: test_given_resource_node_with_multiple_link_children_matching_url_pattern_can_create_new_group_to_bundle_those_links_together') +async def test_given_resource_node_with_multiple_link_children_bundled_as_a_group_can_easily_forget_the_group_to_unbundle_the_links() -> None: + pass + + +# === Utility === + +# NOTE: Only for use with tree items in EntityTree +_assert_tree_item_icon_tooltip_contains = EntityTree.assert_tree_item_icon_tooltip_contains diff --git a/src/crystal/tests/test_addrooturl.py b/src/crystal/tests/test_addrooturl.py index e9ef7efd..d4dd264e 100644 --- a/src/crystal/tests/test_addrooturl.py +++ b/src/crystal/tests/test_addrooturl.py @@ -2,11 +2,18 @@ from crystal.browser.addrooturl import fields_hide_hint_when_focused from crystal.model import Project, Resource, RootResource from crystal.tests.util.asserts import * -from crystal.tests.util.controls import click_button +from crystal.tests.util.controls import click_button, TreeItem +from crystal.tests.util.server import served_project from crystal.tests.util.subtests import awith_subtests, SubtestsContext, with_subtests -from crystal.tests.util.wait import DEFAULT_WAIT_PERIOD, wait_for -from crystal.tests.util.windows import AddUrlDialog, OpenOrCreateDialog +from crystal.tests.util.tasks import 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, + wait_for, +) +from crystal.tests.util.windows import AddUrlDialog, EntityTree, OpenOrCreateDialog from crystal.util.wx_window import SetFocus +from crystal.util.xos import is_windows import crystal.url_input from crystal.url_input import _candidate_urls_from_user_input as EXPAND import os @@ -19,6 +26,188 @@ import wx +# === Test: Create & Delete Standalone === + +async def test_can_create_root_url(*, ensure_revisions_not_deleted: bool=False) -> None: + with served_project('testdata_xkcd.crystalproj.zip') as sp: + # Define URLs + home_url = sp.get_request_url('https://xkcd.com/') + + async with (await OpenOrCreateDialog.wait_for()).create() as (mw, _): + # Create root URL + if True: + root_ti = TreeItem.GetRootItem(mw.entity_tree.window) + assert root_ti is not None + () = root_ti.Children + + assert mw.add_url_button.Enabled + click_button(mw.add_url_button) + aud = await AddUrlDialog.wait_for() + + # Ensure prepopulates reasonable information + assert '' == aud.url_field.Value + assert '' == aud.name_field.Value + #assert None == agd.source + assert aud.url_field.HasFocus # default focused field + + aud.name_field.Value = 'Home' + aud.url_field.Value = home_url + await aud.ok() + + # Ensure appearance is correct + (home_ti,) = [ + child for child in root_ti.Children + if child.Text.startswith(f'{home_url} - ') + ] + assert f'{home_url} - Home' == home_ti.Text + await _assert_tree_item_icon_tooltip_contains(home_ti, 'root URL') + await _assert_tree_item_icon_tooltip_contains(home_ti, 'Undownloaded') + + # Currently, an entirely new root URL is NOT selected automatically. + # This behavior might be changed in the future. + if not is_windows(): + selected_ti = TreeItem.GetSelection(mw.entity_tree.window) + assert (selected_ti is None) or (selected_ti == root_ti) + + if ensure_revisions_not_deleted: + # Download a revision of the root URL + home_ti.SelectItem() + await mw.click_download_button() + await wait_for_download_to_start_and_finish(mw.task_tree) + await _assert_tree_item_icon_tooltip_contains(home_ti, 'Fresh') + + # Forget root URL + if True: + home_ti.SelectItem() + assert mw.forget_button.IsEnabled() + click_button(mw.forget_button) + + # Ensure cannot find root URL + () = [ + child for child in root_ti.Children + if child.Text.startswith(f'{home_url} - ') + ] + if not is_windows(): + selected_ti = TreeItem.GetSelection(mw.entity_tree.window) + assert (selected_ti is None) or (selected_ti == root_ti) + + if ensure_revisions_not_deleted: + # Recreate the root URL + click_button(mw.add_url_button) + aud = await AddUrlDialog.wait_for() + aud.name_field.Value = 'Home' + aud.url_field.Value = home_url + await aud.ok() + + # 1. Ensure appearance is correct + # 2. Ensure previously downloaded revisions still exist + (home_ti,) = [ + child for child in root_ti.Children + if child.Text.startswith(f'{home_url} - ') + ] + assert f'{home_url} - Home' == home_ti.Text + await _assert_tree_item_icon_tooltip_contains(home_ti, 'root URL') + await _assert_tree_item_icon_tooltip_contains(home_ti, 'Fresh') + + +@skip('covered by: test_can_create_root_url') +async def test_can_forget_root_url() -> None: + pass + + +async def test_when_forget_root_url_then_revisions_of_that_url_are_not_deleted() -> None: + await test_can_create_root_url(ensure_revisions_not_deleted=True) + + +# === Test: Create & Delete from Links === + +async def test_given_resource_node_with_links_can_create_new_root_url_to_label_link() -> None: + with served_project('testdata_xkcd.crystalproj.zip') as sp: + # Define URLs + home_url = sp.get_request_url('https://xkcd.com/') + atom_feed_url = sp.get_request_url('https://xkcd.com/atom.xml') + + async with (await OpenOrCreateDialog.wait_for()).create() as (mw, _): + # Create home URL + if True: + root_ti = TreeItem.GetRootItem(mw.entity_tree.window) + assert root_ti is not None + () = root_ti.Children + + assert mw.add_url_button.Enabled + click_button(mw.add_url_button) + aud = await AddUrlDialog.wait_for() + + aud.name_field.Value = 'Home' + aud.url_field.Value = home_url + await aud.ok() + (home_ti,) = root_ti.Children + + # Expand home URL + home_ti.Expand() + await wait_for_download_to_start_and_finish(mw.task_tree) + assert first_child_of_tree_item_is_not_loading_condition(home_ti)() + + # Select the Atom Feed link from the home URL + (atom_feed_ti,) = [ + child for child in home_ti.Children + if child.Text.startswith(f'{atom_feed_url} - ') + ] # ensure did find sub-resource + atom_feed_ti.SelectItem() + + # Create a root resource to label the link + if True: + assert mw.add_url_button.Enabled + click_button(mw.add_url_button) + aud = await AddUrlDialog.wait_for() + + # Ensure prepopulates reasonable information + assert atom_feed_url == aud.url_field.Value # default pattern = (from resource) + assert 'Feed' == aud.name_field.Value # default name = (from first text link) + #assert 'Home' == aud.source # default source = (from resource parent) + assert aud.url_field.HasFocus # default focused field + + # Input new name + aud.name_field.Value = 'Atom Feed' + + await aud.ok() + + # 1. Ensure the new root resource does now label the link + # 2. Ensure the labeled link is selected immediately after closing the add URL dialog + (atom_feed_ti,) = [ + child for child in home_ti.Children + if child.Text.startswith(f'{atom_feed_url} - ') + ] # ensure did find sub-resource + assert ( + # title format of labeled sub-resource + f'{atom_feed_url} - Atom Feed' == + atom_feed_ti.Text) + assert atom_feed_ti.IsSelected() + + # Forget the root resource to unlabel the link + if True: + assert atom_feed_ti.IsSelected() + assert mw.forget_button.IsEnabled() + click_button(mw.forget_button) + + # 1. Ensure can find the unlabeled link + # 2. Ensure that unlabeled link is selected immediately after forgetting the root resource + (atom_feed_ti,) = [ + child for child in home_ti.Children + if child.Text.startswith(f'{atom_feed_url} - ') + ] # ensure did find sub-resource + assert ( + # title format of unlabeled sub-resource + f'{atom_feed_url} - Unknown Link (rel=alternate), Link: Feed, Link: Atom Feed' == + atom_feed_ti.Text) + assert atom_feed_ti.IsSelected() + + +@skip('covered by: test_given_resource_node_with_links_can_create_new_root_url_to_label_link') +async def test_given_resource_node_with_link_labeled_as_root_url_can_easily_forget_the_root_url_to_unlabel_the_link() -> None: + pass + + # === Test: URL Input -> Candidate URLs === def test_given_empty_string_then_returns_empty_string() -> None: @@ -694,3 +883,7 @@ def _assert_contains_sublist(xs: List[str], ys: List[str]) -> None: last_index = xs.index(y, last_index + 1) except IndexError: raise AssertionError(f'Expected list {xs} to contain sublist {ys} but it does not') + + +# NOTE: Only for use with tree items in EntityTree +_assert_tree_item_icon_tooltip_contains = EntityTree.assert_tree_item_icon_tooltip_contains diff --git a/src/crystal/tests/test_workflows.py b/src/crystal/tests/test_workflows.py index 9c2be20a..fd134cd1 100644 --- a/src/crystal/tests/test_workflows.py +++ b/src/crystal/tests/test_workflows.py @@ -23,6 +23,7 @@ AddGroupDialog, AddUrlDialog, EntityTree, MainWindow, OpenOrCreateDialog, PreferencesDialog, ) +from crystal.util.xos import is_windows import datetime import re import tempfile @@ -89,7 +90,7 @@ async def test_can_download_and_serve_a_static_site() -> None: assert True == (await is_url_not_in_archive(home_url)) # Test can download root resource (using download button) - assert home_ti.id == mw.entity_tree.window.GetSelection() + assert home_ti.IsSelected() await mw.click_download_button() await wait_for_download_to_start_and_finish(mw.task_tree) @@ -361,9 +362,9 @@ async def test_can_download_and_serve_a_static_site() -> None: # 1. Test cannot add new root resource in read-only project # 2. Test cannot add new resource group in read-only project - selected_ti = TreeItem.GetSelection(mw.entity_tree.window) - # NOTE: Cannot test the selection on Windows - #assert (selected_ti is None) or (selected_ti == root_ti) + if not is_windows(): + selected_ti = TreeItem.GetSelection(mw.entity_tree.window) + assert (selected_ti is None) or (selected_ti == root_ti) assert False == mw.add_url_button.IsEnabled() assert False == mw.add_group_button.IsEnabled() @@ -1184,10 +1185,7 @@ async def _undiscover_url( # NOTE: Only for use with tree items in EntityTree -async def _assert_tree_item_icon_tooltip_contains(ti: TreeItem, value: str) -> None: - tooltip = await (EntityTree(ti.tree).get_tree_item_icon_tooltip(ti)) - assert tooltip is not None and value in tooltip, \ - f'Expected tooltip to contain {value!r}, but it was: {tooltip!r}' +_assert_tree_item_icon_tooltip_contains = EntityTree.assert_tree_item_icon_tooltip_contains # ------------------------------------------------------------------------------ diff --git a/src/crystal/tests/util/controls.py b/src/crystal/tests/util/controls.py index fc5a1b29..a722bf22 100644 --- a/src/crystal/tests/util/controls.py +++ b/src/crystal/tests/util/controls.py @@ -1,6 +1,7 @@ from __future__ import annotations from contextlib import contextmanager +from crystal.util.xos import is_windows from typing import Iterator, List, Optional import unittest.mock import wx @@ -128,8 +129,15 @@ def Children(self) -> List[TreeItem]: return get_children_of_tree_item(self.tree, self.id) def __eq__(self, other: object) -> bool: - # wx.TreeItemId does not support equality comparison on Windows - return NotImplemented + if is_windows(): + # wx.TreeItemId does not support equality comparison on Windows + raise ValueError('Cannot compare TreeItems on Windows') + else: + return ( + isinstance(other, TreeItem) and + self.tree == other.tree and + self.id == other.id + ) # ------------------------------------------------------------------------------ diff --git a/src/crystal/tests/util/windows.py b/src/crystal/tests/util/windows.py index bb51caca..dd4d515c 100644 --- a/src/crystal/tests/util/windows.py +++ b/src/crystal/tests/util/windows.py @@ -358,6 +358,8 @@ async def cancel(self) -> None: class AddGroupDialog: + _NONE_SOURCE_TITLE = 'none' + name_field: wx.TextCtrl pattern_field: wx.TextCtrl source_field: wx.Choice @@ -403,8 +405,13 @@ def _get_source(self) -> Optional[str]: selection_ci = self.source_field.GetSelection() if selection_ci == wx.NOT_FOUND: return None - return self.source_field.GetString(selection_ci) - def _set_source(self, source_title: str) -> None: + source_title = self.source_field.GetString(selection_ci) + if source_title == self._NONE_SOURCE_TITLE: + return None + return source_title + def _set_source(self, source_title: Optional[str]) -> None: + if source_title is None: + source_title = self._NONE_SOURCE_TITLE selection_ci = self.source_field.GetStrings().index(source_title) self.source_field.SetSelection(selection_ci) source = property(_get_source, _set_source) @@ -480,6 +487,12 @@ async def get_tree_item_icon_tooltip(self, tree_item: TreeItem) -> Optional[str] raise AssertionError('GetTooltipEvent did not return tooltip') return event.tooltip_cell[0] + @staticmethod + async def assert_tree_item_icon_tooltip_contains(ti: TreeItem, value: str) -> None: + tooltip = await (EntityTree(ti.tree).get_tree_item_icon_tooltip(ti)) + assert tooltip is not None and value in tooltip, \ + f'Expected tooltip to contain {value!r}, but it was: {tooltip!r}' + async def set_default_url_prefix_to_resource_at_tree_item(self, tree_item: TreeItem) -> None: # TODO: Publicize constant from crystal.browser.entitytree import _ID_SET_PREFIX From 161bff59d5dd76836b522c65bc75326df7eb3da3 Mon Sep 17 00:00:00 2001 From: David Foster Date: Sun, 7 Jan 2024 11:21:37 -0500 Subject: [PATCH 6/8] serve: Don't invalidate served XML files by inserting