diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index b57e0842..b87b6fa4 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -10,6 +10,13 @@ Release Notes ⋮ [high-priority issues]: https://github.com/davidfstr/Crystal-Web-Archiver/issues?q=is%3Aopen+is%3Aissue+label%3Apriority-high [medium-priority issues]: https://github.com/davidfstr/Crystal-Web-Archiver/issues?q=is%3Aopen+is%3Aissue+label%3Apriority-medium +### main (v2.0.0?) + +* First-time-run experience improvements + * Add Root URL, Add Group: + * Make it optional to provide a name for new Root URLs and Groups. + * Rearrange fields to deemphasize the Name field. + ### v1.7.0b (December 18, 2023) This release features further improvements to downloading large websites diff --git a/src/crystal/browser/addgroup.py b/src/crystal/browser/addgroup.py index 76773140..043592ce 100644 --- a/src/crystal/browser/addgroup.py +++ b/src/crystal/browser/addgroup.py @@ -136,9 +136,9 @@ def _create_fields(self, name='cr-add-group-dialog__source-field') self.source_choice_box.Append('none', None) for rr in self._project.root_resources: - self.source_choice_box.Append(rr.name, rr) + self.source_choice_box.Append(rr.display_name, rr) for rg in self._project.resource_groups: - self.source_choice_box.Append(rg.name, rg) + self.source_choice_box.Append(rg.display_name, rg) self.source_choice_box.SetSelection(0) if initial_source is not None: for i in range(self.source_choice_box.GetCount()): diff --git a/src/crystal/browser/addrooturl.py b/src/crystal/browser/addrooturl.py index b80f81ec..cd35d23a 100644 --- a/src/crystal/browser/addrooturl.py +++ b/src/crystal/browser/addrooturl.py @@ -65,7 +65,7 @@ def _create_fields(self, parent: wx.Window, initial_url: str) -> wx.Sizer: self.name_field = wx.TextCtrl( parent, name='cr-add-url-dialog__name-field') - self.url_field.Hint = 'Home' + self.name_field.Hint = 'Home' self.name_field.SetSelection(-1, -1) # select all upon focus fields_sizer.Add(self.name_field, flag=wx.EXPAND) diff --git a/src/crystal/browser/entitytree.py b/src/crystal/browser/entitytree.py index 97f1ef79..8558b05d 100644 --- a/src/crystal/browser/entitytree.py +++ b/src/crystal/browser/entitytree.py @@ -709,9 +709,11 @@ def _entity_tooltip(self) -> str: def calculate_title(self): project = self.root_resource.project - return '%s - %s' % ( - project.get_display_url(self.root_resource.url), - self.root_resource.name) + display_url = project.get_display_url(self.root_resource.url) + if self.root_resource.name != '': + return '%s - %s' % (display_url, self.root_resource.name) + else: + return '%s' % (display_url,) @property def entity(self): @@ -846,9 +848,11 @@ def icon_tooltip(self) -> Optional[str]: def calculate_title(self): project = self.resource_group.project - return '%s - %s' % ( - project.get_display_url(self.resource_group.url_pattern), - self.resource_group.name) + display_url = project.get_display_url(self.resource_group.url_pattern) + if self.resource_group.name != '': + return '%s - %s' % (display_url, self.resource_group.name) + else: + return '%s' % (display_url,) @property def entity(self): @@ -970,10 +974,17 @@ def icon_tooltip(self) -> Optional[str]: def calculate_title(self): project = self.resource_group.project - return '%s - %d of %s' % ( - project.get_display_url(self.resource_group.url_pattern), - len(self.children), - self.resource_group.name) + display_url_pattern = project.get_display_url(self.resource_group.url_pattern) + if self.resource_group.name != '': + return '%s - %d of %s' % ( + display_url_pattern, + len(self.children), + self.resource_group.name) + else: + return '%s - %d link%s' % ( + display_url_pattern, + len(self.children), + '' if len(self.children) == 1 else 's') @property def entity(self): diff --git a/src/crystal/model.py b/src/crystal/model.py index c89a76a1..a03e79ca 100644 --- a/src/crystal/model.py +++ b/src/crystal/model.py @@ -1792,7 +1792,7 @@ def definitely_has_no_revisions(self) -> bool: """ return self._definitely_has_no_revisions - # === Download === + # === Operations: Download === def download_body(self) -> 'Future[ResourceRevision]': """ @@ -2104,13 +2104,15 @@ class RootResource: resource: Resource _id: int # or None if deleted + # === Init === + def __new__(cls, project: Project, name: str, resource: Resource, _id: Optional[int]=None) -> RootResource: """ Creates a new root resource. Arguments: * project -- associated `Project`. - * name -- display name. + * name -- name. Possibly ''. * resource -- `Resource`. Raises: @@ -2148,6 +2150,8 @@ def __new__(cls, project: Project, name: str, resource: Resource, _id: Optional[ return self + # === Delete === + def delete(self) -> None: """ Deletes this root resource. @@ -2166,10 +2170,19 @@ def delete(self) -> None: del self.project._root_resources[self.resource] + # === Properties === + + @property + def display_name(self) -> str: + """Name of this root resource that is used in the UI.""" + return self.name or self.url + @property def url(self) -> str: return self.resource.url + # === Operations: Download === + # TODO: Create the underlying task with the full RootResource # so that the correct subtitle is displayed. def download(self, *, needs_result: bool=True) -> Future[ResourceRevision]: @@ -2185,6 +2198,8 @@ def create_download_task(self, *, needs_result: bool=True) -> Task: # so that the correct subtitle is displayed. return self.resource.create_download_task(needs_result=needs_result, is_embedded=False) + # === Utility === + def __repr__(self): return "RootResource(%s,%s)" % (repr(self.name), repr(self.resource.url)) @@ -3079,19 +3094,24 @@ class ResourceGroup(ListenableMixin): Persisted and auto-saved. """ + # === Init === + def __init__(self, project: Project, - name: str, + name: str, # possibly '' url_pattern: str, _id: Optional[int]=None) -> None: """ Arguments: * project -- associated `Project`. - * name -- name of this group. + * name -- name of this group. Possibly ''. * url_pattern -- url pattern matched by this group. """ super().__init__() + if len(url_pattern) == 0: + raise ValueError('Cannot create group with empty pattern') + self.project = project self.name = name self.url_pattern = url_pattern @@ -3118,6 +3138,8 @@ def __init__(self, def _init_source(self, source: ResourceGroupSource) -> None: self._source = source + # === Delete === + def delete(self) -> None: """ Deletes this resource group. @@ -3136,6 +3158,13 @@ def delete(self) -> None: self.project._resource_groups.remove(self) + # === Properties === + + @property + def display_name(self) -> str: + """Name of this group that is used in the UI.""" + return self.name or self.url_pattern + def _get_source(self) -> ResourceGroupSource: """ The "source" of this resource group. @@ -3234,6 +3263,8 @@ def members(self) -> 'Sequence[Resource]': literal_prefix=ResourceGroup.literal_prefix_for_url_pattern(self.url_pattern)) return self._members + # === Events === + # Called when a new Resource is created after the project has loaded def _resource_did_instantiate(self, resource: Resource) -> None: if self.contains_url(resource.url): @@ -3260,6 +3291,8 @@ def _resource_will_delete(self, resource: Resource) -> None: except ValueError: # not in list pass + # === Operations: Download === + def download(self, *, needs_result: bool=False) -> DownloadResourceGroupTask: """ Downloads this group asynchronously. @@ -3303,6 +3336,8 @@ def update_membership(self) -> None: from crystal.task import UpdateResourceGroupMembersTask task = UpdateResourceGroupMembersTask(self) self.project.add_task(task) + + # === Utility === def __repr__(self): return 'ResourceGroup(%s,%s)' % (repr(self.name), repr(self.url_pattern)) diff --git a/src/crystal/server.py b/src/crystal/server.py index 07f8f665..382696f5 100644 --- a/src/crystal/server.py +++ b/src/crystal/server.py @@ -416,7 +416,7 @@ def _do_GET(self) -> Generator[SwitchToThread, None, None]: matching_rg = self._find_group_matching_archive_url(archive_url) if matching_rg is not None and not readonly and dynamic_ok: self._print_warning('*** Dynamically downloading new resource in group %r: %s' % ( - matching_rg.name, + matching_rg.display_name, archive_url, )) @@ -453,7 +453,7 @@ def get_default_revision() -> Optional[ResourceRevision]: matching_rr = self._find_root_resource_matching_archive_url(archive_url) if matching_rr is not None: self._print_warning('*** Dynamically downloading root resource %r: %s' % ( - matching_rr.name, + matching_rr.display_name, archive_url, )) @@ -463,7 +463,7 @@ def get_default_revision() -> Optional[ResourceRevision]: matching_rg = self._find_group_matching_archive_url(archive_url) if matching_rg is not None: self._print_warning('*** Dynamically downloading existing resource in group %r: %s' % ( - matching_rg.name, + matching_rg.display_name, archive_url, )) diff --git a/src/crystal/task.py b/src/crystal/task.py index cbe1620a..bd4b85bb 100644 --- a/src/crystal/task.py +++ b/src/crystal/task.py @@ -586,14 +586,15 @@ class TaskDisposedException(Exception): _MAX_EMBEDDED_RESOURCE_RECURSION_DEPTH = 3 -def _get_abstract_resource_title(abstract_resource): +def _get_abstract_resource_title(abstract_resource: Union[Resource, RootResource]) -> str: """ Arguments: * abstract_resource -- a Resource or a RootResource. """ resource = abstract_resource.resource - if hasattr(abstract_resource, 'name'): - return '%s - %s' % (resource.url, abstract_resource.name) + name = getattr(abstract_resource, 'name', '') or '' + if name != '': + return '%s - %s' % (resource.url, name) else: return '%s' % (resource.url) @@ -731,7 +732,11 @@ class DownloadResourceTask(Task): '_download_body_with_embedded_future', ) - def __init__(self, abstract_resource, *, needs_result: bool=True, is_embedded: bool=False) -> None: + def __init__(self, + abstract_resource: Union[Resource, RootResource], + *, needs_result: bool=True, + is_embedded: bool=False, + ) -> None: """ Arguments: * abstract_resource -- a Resource or a RootResource. @@ -1189,7 +1194,7 @@ class UpdateResourceGroupMembersTask(Task): def __init__(self, group: ResourceGroup) -> None: super().__init__( - title='Finding members of group: %s' % group.name) + title='Finding members of group: %s' % group.display_name) self.group = group if group.source is None: @@ -1233,7 +1238,7 @@ class DownloadResourceGroupMembersTask(Task): def __init__(self, group: ResourceGroup) -> None: super().__init__( - title='Downloading members of group: %s' % group.name) + title='Downloading members of group: %s' % group.display_name) self.group = group if self._use_extra_listener_assertions: @@ -1377,7 +1382,7 @@ class DownloadResourceGroupTask(Task): def __init__(self, group: ResourceGroup) -> None: super().__init__( - title='Downloading group: %s' % group.name) + title='Downloading group: %s' % group.display_name) self._update_members_task = UpdateResourceGroupMembersTask(group) self._download_members_task = DownloadResourceGroupMembersTask(group) self._started_downloading_members = False diff --git a/src/crystal/tests/test_workflows.py b/src/crystal/tests/test_workflows.py index 8df29ed9..8316886e 100644 --- a/src/crystal/tests/test_workflows.py +++ b/src/crystal/tests/test_workflows.py @@ -1017,6 +1017,116 @@ async def test_can_download_and_serve_a_site_requiring_cookie_authentication() - pass +async def test_can_download_a_static_site_with_unnamed_root_urls_and_groups() -> None: + """ + Test that can successfully download a site without needing to name any + Root URLs or Groups, and those unnamed entities look okay everywhere + in the UI. + """ + 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/#/') + + atom_feed_url = sp.get_request_url('https://xkcd.com/atom.xml') + rss_feed_url = sp.get_request_url('https://xkcd.com/rss.xml') + feed_pattern = sp.get_request_url('https://xkcd.com/*.xml') + + feed_item_pattern = sp.get_request_url('https://xkcd.com/##/') + assert feed_item_pattern != comic_pattern + + with tempfile.TemporaryDirectory(suffix='.crystalproj') as project_dirpath: + async with (await OpenOrCreateDialog.wait_for()).create(project_dirpath) as (mw, _): + # 1. Test can create unnamed root resource + # 2. Ensure unnamed root resource at root of Entity Tree has OK title + if True: + root_ti = TreeItem.GetRootItem(mw.entity_tree.window) + assert root_ti is not None + assert root_ti.GetFirstChild() is None # no entities + + click_button(mw.add_url_button) + aud = await AddUrlDialog.wait_for() + + aud.url_field.Value = home_url + await aud.ok() + home_ti = root_ti.GetFirstChild() + assert home_ti is not None # entity was created + assert f'{home_url}' == home_ti.Text + + # Expand root resource + home_ti.Expand() + await wait_for(first_child_of_tree_item_is_not_loading_condition(home_ti)) + (comic1_ti,) = [ + child for child in home_ti.Children + if child.Text.startswith(f'{comic1_url} - ') + ] # ensure did find sub-resource for Comic #1 + + # 1. Test can create unnamed resource group + # 2. Ensure unnamed root resource shows as source with OK title + if True: + comic1_ti.SelectItem() + + click_button(mw.add_group_button) + agd = await AddGroupDialog.wait_for() + + agd.pattern_field.Value = comic_pattern + agd.source = home_url + await agd.ok() + + # 1. Ensure the new resource group does now group sub-resources + # 2. Ensure grouped sub-resources for unnamed group has OK title + 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+ links?', # title format of grouped sub-resources + grouped_subresources_ti.Text) + + grouped_subresources_ti.Expand() + await wait_for(first_child_of_tree_item_is_not_loading_condition(grouped_subresources_ti)) + + (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 + + grouped_subresources_ti.Collapse() + + home_ti.Collapse() + + # 1. Ensure the new resource group appears at the root of the entity tree + # 2. Ensure unnamed resource group at root of Entity Tree has OK title + (comic_group_ti,) = [ + child for child in root_ti.Children + if child.Text == f'{comic_pattern}' + ] # ensure did find resource group at root of entity tree + + comic_group_ti.Expand() + await wait_for(first_child_of_tree_item_is_not_loading_condition(comic_group_ti)) + + # Ensure the new resource group does contain the expected members + (comic1_ti,) = [ + child for child in comic_group_ti.Children + if child.Text == f'{comic1_url}' + ] # contains first comic + assert len(comic_group_ti.Children) >= 2 # contains last comic too + + comic_group_ti.Collapse() + + # Ensure unnamed resource group shows as source with OK title + click_button(mw.add_group_button) + agd = await AddGroupDialog.wait_for() + agd.source = comic_pattern + click_button(agd.cancel_button) + + # ------------------------------------------------------------------------------ # Utility