From 77ae84fa67697158911112fbb9b60e3832c6f479 Mon Sep 17 00:00:00 2001 From: David Foster Date: Sun, 21 Jan 2024 09:13:59 -0500 Subject: [PATCH] Fix delete of ResourceGroup to update do-not-download badges in EntityTree properly --- src/crystal/browser/__init__.py | 4 -- src/crystal/browser/entitytree.py | 29 +++++++++---- src/crystal/model.py | 43 ++++++++++++++----- .../tests/test_do_not_download_groups.py | 10 +++-- 4 files changed, 61 insertions(+), 25 deletions(-) diff --git a/src/crystal/browser/__init__.py b/src/crystal/browser/__init__.py index c8280834..06d216b8 100644 --- a/src/crystal/browser/__init__.py +++ b/src/crystal/browser/__init__.py @@ -574,10 +574,6 @@ def _on_forget_entity(self, event) -> None: assert selected_or_related_entity is not None selected_or_related_entity.delete() - - # TODO: This update() should happen in response to an event - # fired by the entity itself. - self.entity_tree.update() def _on_download_entity(self, event) -> None: selected_entity = self.entity_tree.selected_entity diff --git a/src/crystal/browser/entitytree.py b/src/crystal/browser/entitytree.py index 731fedeb..fdeee564 100644 --- a/src/crystal/browser/entitytree.py +++ b/src/crystal/browser/entitytree.py @@ -131,14 +131,16 @@ def name_of_selection(self) -> Optional[str]: # === Updates === + # TODO: Can this be marked with @fg_affinity? def update(self): """ Updates the nodes in this tree, usually due to a project change. """ self.root.update_descendants() - # === Event: Resource Did Instantiate === + # === Events: Resource Lifecycle === + # TODO: Can this be marked with @fg_affinity? def resource_did_instantiate(self, resource: Resource) -> None: # TODO: Optimize to only refresh those groups that could potentially # be affected by this particular resource being instantiated @@ -160,19 +162,22 @@ def _refresh_group_nodes_now(self) -> None: finally: self._group_nodes_need_updating = False - # === Event: Resource Revision Did Instantiate === + # === Events: Resource Revision Lifecycle === + # TODO: Can this be marked with @fg_affinity? Then remove interior fg_call_later(). def resource_revision_did_instantiate(self, revision: ResourceRevision) -> None: fg_call_later(lambda: self.root.update_icon_set_of_descendants_with_resource(revision.resource)) - # === Event: Root Resource Did Instantiate === + # === Events: Root Resource Lifecycle === + # TODO: Can this be marked with @fg_affinity? def root_resource_did_instantiate(self, root_resource: RootResource) -> None: self.update() - # === Event: Resource Group Did Instantiate === + # === Events: Resource Group Lifecycle === + # TODO: Can this be marked with @fg_affinity? Then remove interior fg_call_later(). def resource_group_did_instantiate(self, group: ResourceGroup) -> None: self.update() @@ -181,11 +186,19 @@ def resource_group_did_instantiate(self, group: ResourceGroup) -> None: fg_call_later(lambda: self.root.update_icon_set_of_descendants_in_group(group)) - # === Event: Resource Group Did Change Do-Not-Download === - + @fg_affinity def resource_group_did_change_do_not_download(self, group: ResourceGroup) -> None: - fg_call_later(lambda: - self.root.update_icon_set_of_descendants_in_group(group)) + self.root.update_icon_set_of_descendants_in_group(group) + + @fg_affinity + def resource_group_will_forget(self, group: ResourceGroup) -> None: + pass + + @fg_affinity + def resource_group_did_forget(self, group: ResourceGroup) -> None: + self.update() + + self.root.update_icon_set_of_descendants_in_group(group) # === Event: Min Fetch Date Did Change === diff --git a/src/crystal/model.py b/src/crystal/model.py index 8be97c90..ed30503c 100644 --- a/src/crystal/model.py +++ b/src/crystal/model.py @@ -1328,7 +1328,7 @@ def add_task(self, task: Task) -> None: if task_was_complete: self.root_task.child_task_did_complete(task) - # === Events === + # === Events: Resource Lifecycle === # Called when a new Resource is created after the project has loaded def _resource_did_instantiate(self, resource: Resource) -> None: @@ -1341,13 +1341,6 @@ def _resource_did_instantiate(self, resource: Resource) -> None: if hasattr(lis, 'resource_did_instantiate'): lis.resource_did_instantiate(resource) # type: ignore[attr-defined] - # Called when a new ResourceRevision is created after the project has loaded - def _resource_revision_did_instantiate(self, revision: ResourceRevision) -> None: - # Notify normal listeners - for lis in self.listeners: - if hasattr(lis, 'resource_revision_did_instantiate'): - lis.resource_revision_did_instantiate(revision) # type: ignore[attr-defined] - def _resource_did_alter_url(self, resource: Resource, old_url: str, new_url: str) -> None: del self._resource_for_url[old_url] @@ -1365,6 +1358,17 @@ def _resource_will_delete(self, resource: Resource) -> None: for rg in self.resource_groups: rg._resource_will_delete(resource) + # === Events: Resource Revision Lifecycle === + + # Called when a new ResourceRevision is created after the project has loaded + def _resource_revision_did_instantiate(self, revision: ResourceRevision) -> None: + # Notify normal listeners + for lis in self.listeners: + if hasattr(lis, 'resource_revision_did_instantiate'): + lis.resource_revision_did_instantiate(revision) # type: ignore[attr-defined] + + # === Events: Root Resource Lifecycle === + # Called when a new RootResource is created after the project has loaded def _root_resource_did_instantiate(self, root_resource: RootResource) -> None: # Notify normal listeners @@ -1372,6 +1376,8 @@ def _root_resource_did_instantiate(self, root_resource: RootResource) -> None: if hasattr(lis, 'root_resource_did_instantiate'): lis.root_resource_did_instantiate(root_resource) # type: ignore[attr-defined] + # === Events: Resource Group Lifecycle === + # Called when a new ResourceGroup is created after the project has loaded def _resource_group_did_instantiate(self, group: ResourceGroup) -> None: # Notify normal listeners @@ -1379,13 +1385,24 @@ def _resource_group_did_instantiate(self, group: ResourceGroup) -> None: if hasattr(lis, 'resource_group_did_instantiate'): lis.resource_group_did_instantiate(group) # type: ignore[attr-defined] - # Called when a new ResourceGroup changes its do-not-download status def _resource_group_did_change_do_not_download(self, group: ResourceGroup) -> None: # Notify normal listeners for lis in self.listeners: if hasattr(lis, 'resource_group_did_change_do_not_download'): lis.resource_group_did_change_do_not_download(group) # type: ignore[attr-defined] + def _resource_group_will_forget(self, group: ResourceGroup) -> None: + # Notify normal listeners + for lis in self.listeners: + if hasattr(lis, 'resource_group_will_forget'): + lis.resource_group_will_forget(group) # type: ignore[attr-defined] + + def _resource_group_did_forget(self, group: ResourceGroup) -> None: + # Notify normal listeners + for lis in self.listeners: + if hasattr(lis, 'resource_group_did_forget'): + lis.resource_group_did_forget(group) # type: ignore[attr-defined] + # === Close === def close(self) -> None: @@ -3212,6 +3229,8 @@ def delete(self) -> None: Deletes this resource group. If it is referenced as a source, it will be replaced with None. """ + self.project._resource_group_will_forget(self) + for rg in self.project.resource_groups: if rg.source == self: rg.source = None @@ -3224,6 +3243,8 @@ def delete(self) -> None: self._id = None # type: ignore[assignment] # intentionally leave exploding None self.project._resource_groups.remove(self) + + self.project._resource_group_did_forget(self) # === Properties === @@ -3288,7 +3309,9 @@ def _set_source(self, value: ResourceGroupSource) -> None: def _get_do_not_download(self) -> bool: return self._do_not_download - def _set_do_not_download(self, value: bool) -> None: + # NOTE: The "save" parameter will be used to control whether the new value + # will be persisted, once persistence support for this option is added + def _set_do_not_download(self, value: bool, *, save: bool=True) -> None: if self._do_not_download == value: return self._do_not_download = value diff --git a/src/crystal/tests/test_do_not_download_groups.py b/src/crystal/tests/test_do_not_download_groups.py index 3b896fe2..196b8a7d 100644 --- a/src/crystal/tests/test_do_not_download_groups.py +++ b/src/crystal/tests/test_do_not_download_groups.py @@ -227,9 +227,13 @@ async def test_then_embedded_resource_in_entity_tree_appears_with_do_not_downloa await _assert_tree_item_icon_tooltip_contains(comic_image_r_ti, 'Ignored') # Test: test_given_embedded_resource_also_in_a_non_do_not_download_group_then_embedded_resource_in_entity_tree_does_not_appear_with_do_not_download_badge - specific_comic_image_rg = ResourceGroup( - project, 'air_gap_2x.png', comic_image_r_url) - await _assert_tree_item_icon_tooltip_contains(comic_image_r_ti, 'Undownloaded') + if True: + specific_comic_image_rg = ResourceGroup( + project, 'air_gap_2x.png', comic_image_r_url) + await _assert_tree_item_icon_tooltip_contains(comic_image_r_ti, 'Undownloaded') + + specific_comic_image_rg.delete() + await _assert_tree_item_icon_tooltip_contains(comic_image_r_ti, 'Ignored') @skip('covered by: test_then_embedded_resource_in_entity_tree_appears_with_do_not_download_badge')