Extensive testing of helper function that creates an app instance fro…

…m forms. Overriding form changed_data to handle tags and volume for dash apps.

apps/forms/base.py

@@ -45,6 +45,13 @@ def _setup_form_fields(self):
         # Handle name
         self.fields["name"].initial = ""
 
+        # Initialize the tags field to existing tags or empty list
+        if self.instance and self.instance.pk:
+            self.instance.refresh_from_db()
+            self._original_tags = list(self.instance.tags.all())
+        else:
+            self._original_tags = []
+
     def _setup_form_helper(self):
         # Create a footer for submit form or cancel
         self.footer = Div(
@@ -83,6 +90,13 @@ def clean_note_on_linkonly_privacy(self):
 
         return note_on_linkonly_privacy
 
+    def clean_tags(self):
+        cleaned_data = super().clean()
+        tags = cleaned_data.get("tags", None)
+        if tags is None:
+            return []
+        return tags
+
     def validate_subdomain(self, subdomain_input):
         # If user did not input subdomain, set it to our standard release name
         if not subdomain_input:
@@ -117,6 +131,16 @@ def validate_subdomain(self, subdomain_input):
 
         return SubdomainTuple(subdomain_input, True)
 
+    @property
+    def changed_data(self):
+        # Override the default changed_data to handle the tags field
+        changed_data = super().changed_data
+        if "tags" in changed_data:
+            new_tags = self.cleaned_data.get("tags", [])
+            if list(new_tags) == self._original_tags:
+                changed_data.remove("tags")
+        return changed_data
+
     class Meta:
         # Specify model to be used
         model = BaseAppInstance

apps/forms/dash.py

@@ -41,6 +41,15 @@ def _setup_form_helper(self):
 
         self.helper.layout = Layout(body, self.footer)
 
+    @property
+    def changed_data(self):
+        # Override the default changed_data to handle that volume not part of this app type.
+        # TODO: Consider adding to all app forms that do not contain volume.
+        changed_data = super().changed_data
+        if "volume" in changed_data:
+            changed_data.remove("volume")
+        return changed_data
+
     class Meta:
         model = DashInstance
         fields = [

apps/helpers.py

@@ -355,7 +355,7 @@ def get_URI(instance):
 
 
 @transaction.atomic
-def create_instance_from_form(form, project, app_slug, app_id=None):
+def create_instance_from_form(form, project, app_slug, app_id=None) -> int:
     """
     Create or update an instance from a form. This function handles both the creation of new instances
     and the updating of existing ones based on the presence of an app_id.
@@ -374,6 +374,9 @@ def create_instance_from_form(form, project, app_slug, app_id=None):
     """
     from .tasks import deploy_resource
 
+    assert form is not None, "This function requires a form object"
+    assert project is not None, "This function requires a project object"
+
     new_app = app_id is None
 
     logger.debug(f"Creating or updating a user app via UI form for app_id={app_id}, new_app={new_app}")
@@ -416,34 +419,37 @@ def create_instance_from_form(form, project, app_slug, app_id=None):
     instance = form.save(commit=False)
 
     # Handle status creation or retrieval
-    # TODO: Status. Check this use.
+    # TODO: Remove after well tested
     # We no longer update the app status based on user actions.
     # status = get_or_create_status(instance, app_id)
 
+    # TODO: Clean up some of the asserts after well tested
     # Retrieve or create the subdomain
     subdomain, created = Subdomain.objects.get_or_create(
         subdomain=subdomain_name, project=project, is_created_by_user=is_created_by_user
     )
+    assert subdomain is not None
+    assert subdomain.subdomain == subdomain_name
+
+    subdomain = Subdomain.objects.get(subdomain=subdomain_name, project=project, is_created_by_user=is_created_by_user)
+    assert subdomain is not None
+    assert subdomain.subdomain == subdomain_name
+    assert subdomain.project.name == project.name, f"2 {subdomain.project} should equal {project.name}"
 
     if not new_app:
-        # Previously: if app_id:
         handle_subdomain_change(instance, subdomain, subdomain_name)
 
     app_slug = handle_shiny_proxy_case(instance, app_slug, app_id)
 
     app = get_app(app_slug)
 
-    # TODO: Status. Check this use:
     setup_instance(instance, subdomain, app, project, user_action)
     id = save_instance_and_related_data(instance, form)
 
     assert id > 0, "The instance id should be a positive int"
 
     if do_deploy:
         logger.debug(f"Now deploying resource app with app_id = {app_id}")
-
-        # TODO: Status. Also set the last user action to Updating
-
         deploy_resource.delay(instance.serialize())
     else:
         logger.debug(f"Not re-deploying this app with app_id = {app_id}")
@@ -471,12 +477,10 @@ def handle_subdomain_change(instance: Any, subdomain: str, subdomain_name: str)
     from .tasks import delete_resource
 
     assert instance is not None, "instance is required"
-    # type(instance) is for example DashInstance
 
-    # assert instance.subdomain is not None, f"subdomain is required but is None for instance {instance.name}"
     if instance.subdomain is None:
-        # TODO: Check this. It is possible for subdomain to be missing.
-        # But is this valid?
+        # The subdomain is not yet created, nothing to do
+        logger.debug("The subdomain is not yet created, nothing to do")
         return
 
     if instance.subdomain.subdomain != subdomain_name:
@@ -507,16 +511,14 @@ def get_app(app_slug):
         raise ValueError(f"App with slug {app_slug} not found")
 
 
-# TODO: Status.
 def setup_instance(instance, subdomain, app, project, user_action=None, is_created_by_user=False):
     instance.subdomain = subdomain
     instance.app = app
     instance.chart = instance.app.chart
     instance.project = project
     instance.owner = project.owner
-    # TODO: Status. Is this unit tested?
     instance.latest_user_action = user_action
-    # TODO: Status. We do not set app status any longer.
+    # TODO: Remove after well tested
     # instance.app_status = status