Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
4994f10
showing advance settings in custom app creation
anondo1969 Oct 29, 2024
27b763b
SS-1176 showing advance settings in custom app creation
anondo1969 Oct 29, 2024
b5e8313
SS-1176 saving the default custom url to the database
anondo1969 Oct 30, 2024
636ff9c
SS-1176 modified saving custom default url to database, added dynamic…
anondo1969 Oct 31, 2024
1425fdb
SS-1176 allowing forward slash at the end of the custom default url
anondo1969 Oct 31, 2024
90f2b98
SS-1176 adding the new custom_default_url field to the CustomAppFormT…
anondo1969 Oct 31, 2024
06b74f8
SS-1176 allowing empty string to be valid
anondo1969 Oct 31, 2024
42c1e9f
SS-1176 included migration file
anondo1969 Oct 31, 2024
b5cf2ed
Merge branch 'develop' into SS-1176-Ability-for-users-to-set-a-defaul…
akochari Nov 1, 2024
758939e
remove unnecessary log message
akochari Nov 1, 2024
60002fc
change the field names and explanations
akochari Nov 1, 2024
b3860ab
SS-1176 separated the url validation part to ensure re-usability
anondo1969 Nov 1, 2024
ad3feae
SS-1176 removed custom_default_url from the redeployment_fields
anondo1969 Nov 1, 2024
dc37d17
SS-1176 used Django URLValidator for the url validation
anondo1969 Nov 4, 2024
7b0ec61
SS-1176 edit url validation error message
anondo1969 Nov 6, 2024
8d560bd
SS-1176 fix pre-commit issues
anondo1969 Nov 6, 2024
9ce5d5b
SS-1176 alllowing unicode letter for url validation
anondo1969 Nov 11, 2024
3527fd0
SS-1176 remove url tests
anondo1969 Nov 11, 2024
6c4b717
Merge branch 'develop' of https://github.com/ScilifelabDataCentre/ser…
anondo1969 Nov 18, 2024
cde08ad
SS-1176 modified the code acccording to the review
anondo1969 Nov 19, 2024
83f9954
SS-1176 adding new migration file
anondo1969 Nov 19, 2024
47476ce
Merge branch 'develop' of https://github.com/ScilifelabDataCentre/ser…
anondo1969 Nov 20, 2024
a9b049b
SS-1176 finishing modifications according to the reviews
anondo1969 Nov 20, 2024
bbcb968
SS-1176 catching console error and fixing e2e test for duplicate proj…
anondo1969 Nov 21, 2024
a329f08
Merge branch 'develop' of https://github.com/ScilifelabDataCentre/ser…
anondo1969 Nov 21, 2024
7d84b18
SS-1176 removing comment from e2e test
anondo1969 Nov 21, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ def deploy_resources(self, request, queryset):

for instance in queryset:
instance.set_k8s_values()
instance.url = get_URI(instance.k8s_values)
instance.url = get_URI(instance)
instance.save(update_fields=["k8s_values", "url"])

deploy_resource.delay(instance.serialize())
Expand Down
27 changes: 27 additions & 0 deletions apps/forms/custom.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
from crispy_forms.bootstrap import Accordion, AccordionGroup, PrependedText
from crispy_forms.layout import HTML, Div, Field, Layout, MultiField
from django import forms
from django.core.exceptions import ValidationError
from django.urls import reverse
from django.utils.safestring import mark_safe

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
from django.urls import reverse

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. Addressed it.

from apps.forms.base import AppBaseForm
from apps.forms.field.common import SRVCommonDivField
Expand All @@ -14,12 +18,23 @@ class CustomAppForm(AppBaseForm):
port = forms.IntegerField(min_value=3000, max_value=9999, required=True)
image = forms.CharField(max_length=255, required=True)
path = forms.CharField(max_length=255, required=False)
default_url_subpath = forms.CharField(max_length=255, required=False, label="Custom URL subpath")

def _setup_form_fields(self):
# Handle Volume field
super()._setup_form_fields()
self.fields["volume"].initial = None

self.fields["default_url_subpath"].widget.attrs.update({"class": "textinput form-control"})
self.fields["default_url_subpath"].help_text = "Specify a non-default start URL if your app requires that."
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
self.fields["default_url_subpath"].help_text = "Specify a non-default start URL if your app requires that."
self.fields["default_url_subpath"].help_text = "Specify a non-default start URL if your app requires that."
apps_url = reverse('portal:apps')

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Modified it.

apps_url = reverse("portal:apps")
self.fields["default_url_subpath"].bottom_help_text = mark_safe(
(
f"<span class='fw-bold'>Note:</span> This changes the URL connected to the Open button for an app"
f" on the Serve <a href='{apps_url}'>Apps & Models</a> page."
)
)

def _setup_form_helper(self):
super()._setup_form_helper()

Expand All @@ -40,6 +55,17 @@ def _setup_form_helper(self):
),
SRVCommonDivField("port", placeholder="8000"),
SRVCommonDivField("image"),
Accordion(
AccordionGroup(
"Advanced settings",
PrependedText(
"default_url_subpath",
mark_safe("<span id='id_custom_default_url_prepend'>Subdomain/</span>"),
template="apps/partials/srv_prepend_append_input_group.html",
),
active=False,
),
),
css_class="card-body",
)
self.helper.layout = Layout(body, self.footer)
Expand Down Expand Up @@ -81,6 +107,7 @@ class Meta:
"port",
"image",
"tags",
"default_url_subpath",
]
labels = {
"note_on_linkonly_privacy": "Reason for choosing the link only option",
Expand Down
20 changes: 16 additions & 4 deletions apps/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -221,12 +221,15 @@ def update_status_time(status_object, status_ts, event_msg=None):
status_object.save(update_fields=["time", "info"])


def get_URI(values):
def get_URI(instance):
values = instance.k8s_values
# Subdomain is empty if app is already deleted
subdomain = values["subdomain"] if "subdomain" in values else ""
URI = f"https://{subdomain}.{values['global']['domain']}"

URI = URI.strip("/")
if hasattr(instance, "default_url_subpath") and instance.default_url_subpath != "":
URI = URI + "/" + instance.default_url_subpath
logger.info("Modified URI by adding custom default url for the custom app: %s", URI)
return URI


Expand Down Expand Up @@ -261,7 +264,16 @@ def create_instance_from_form(form, project, app_slug, app_id=None):
do_deploy = True
else:
# Only re-deploy existing apps if one of the following fields was changed:
redeployment_fields = ["subdomain", "volume", "path", "flavor", "port", "image", "access", "shiny_site_dir"]
redeployment_fields = [
"subdomain",
"volume",
"path",
"flavor",
"port",
"image",
"access",
"shiny_site_dir",
]
logger.debug(f"An existing app has changed. The changed form fields: {form.changed_data}")

# Because not all forms contain all fields, we check if the supposedly changed field
Expand Down Expand Up @@ -357,5 +369,5 @@ def save_instance_and_related_data(instance, form):
instance.save()
form.save_m2m()
instance.set_k8s_values()
instance.url = get_URI(instance.k8s_values)
instance.url = get_URI(instance)
instance.save(update_fields=["k8s_values", "url"])
23 changes: 23 additions & 0 deletions apps/migrations/0018_customappinstance_default_url_subpath.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# Generated by Django 5.1.1 on 2024-11-19 13:27

import apps.models.app_types.custom.custom
from django.db import migrations, models


class Migration(migrations.Migration):
dependencies = [
("apps", "0017_alter_streamlitinstance_port"),
]

operations = [
migrations.AddField(
model_name="customappinstance",
name="default_url_subpath",
field=models.CharField(
blank=True,
default="",
max_length=255,
validators=[apps.models.app_types.custom.custom.validate_default_url_subpath],
),
),
]
29 changes: 29 additions & 0 deletions apps/models/app_types/custom/custom.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,40 @@
import regex as re
from django.core.exceptions import ValidationError
Comment on lines +1 to +2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
import regex as re
from django.core.exceptions import ValidationError
import regex as re
from django.core.exceptions import ValidationError

This should be caught by isort I think

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. Addressed it.

from django.db import models

from apps.models import AppInstanceManager, BaseAppInstance

from .base import AbstractCustomAppInstance


def validate_default_url_subpath(candidate):
"""
Validates a custom default url path addition.
The RegexValidator will raise a ValidationError if the input does not match the regular expression.
It is up to the caller to handle the raised exception if desired.
"""
error_message = (
"Your custom URL subpath is not valid, please correct it. "
"It must be 1-53 characters long."
" It can contain only Unicode letters, digits, hyphens"
" ( - ), forward slashes ( / ), and underscores ( _ )."
" It cannot start or end with a hyphen ( - ) and "
"cannot start with a forward slash ( / )."
" It cannot contain consecutive forward slashes ( // )."
)

pattern = r"^(?!-)(?!/)(?!.*//)[\p{Letter}\p{Mark}0-9-/_]{1,53}(?<!-)$|^$"

if not re.match(pattern, candidate):
raise ValidationError(error_message)


class CustomAppInstanceManager(AppInstanceManager):
model_type = "customappinstance"


class CustomAppInstance(AbstractCustomAppInstance, BaseAppInstance):
default_url_subpath = models.CharField(
validators=[validate_default_url_subpath], max_length=255, default="", blank=True
)
objects = CustomAppInstanceManager()
48 changes: 48 additions & 0 deletions apps/tests/test_forms.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
import pytest
from django.contrib.auth import get_user_model
from django.core.exceptions import ValidationError
from django.template import Context, Template
from django.test import TestCase

from apps.forms import CustomAppForm
from apps.models import Apps, AppStatus, Subdomain, VolumeInstance
from apps.models.app_types.custom.custom import validate_default_url_subpath
from projects.models import Flavor, Project

User = get_user_model()
Expand Down Expand Up @@ -44,6 +47,7 @@ def setUp(self):
"port": 8000,
"image": "ghcr.io/scilifelabdatacentre/image:tag",
"tags": ["tag1", "tag2", "tag3"],
"default_url_subpath": "valid-default_url_subpath/",
}

def test_form_valid_data(self):
Expand Down Expand Up @@ -192,3 +196,47 @@ def test_form_rendering(self):
self.assertIn('value="private"', rendered_form)
self.assertIn('value="link"', rendered_form)
self.assertIn('value="public"', rendered_form)


invalid_default_url_subpath_list = [
"invalid space",
'invalid_"_double_quote',
"invalid_<_less_than_sign",
"invalid_\\_backslash",
"invalid_|_pipe",
"invalid_^_caret",
"invalid_{_left_curly_brace",
"invalid_?_question_mark",
]

valid_default_url_subpath_list = [
"valid_ÄÄ_unicode_charecters",
"valid_aa/_forward_slash",
"valid_____underscore",
"_aa/bb/c_format",
"valid_-_hiphen",
"_ad-frt/fgh_cd_",
"ÅÄaad1234",
]


@pytest.mark.parametrize("valid_default_url_subpath", valid_default_url_subpath_list)
def test_valid_default_url_subpath(valid_default_url_subpath):
valid_check = True
try:
validate_default_url_subpath(valid_default_url_subpath)
except ValidationError:
valid_check = False

assert valid_check


@pytest.mark.parametrize("invalid_default_url_subpath", invalid_default_url_subpath_list)
def test_invalid_default_url_subpath(invalid_default_url_subpath):
valid_check = True
try:
validate_default_url_subpath(invalid_default_url_subpath)
except ValidationError:
valid_check = False

assert not valid_check
10 changes: 5 additions & 5 deletions cypress/e2e/ui-tests/test-project-as-contributor.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ describe("Test project contributor user functionality", () => {

it("limit on number of apps per project is enforced", () => {
// Names of objects to create
const project_name = "e2e-create-proj-test"
const project_name = "e2e-create-proj-test-1"

// Create a project
cy.visit("/projects/")
Expand Down Expand Up @@ -268,14 +268,14 @@ describe("Test project contributor user functionality", () => {

it("limit on number of projects per user is enforced", () => {
// Names of projects to create
const project_name = "e2e-create-proj-test"
const project_name = "e2e-create-proj-test-2"

// Create 10 projects (current limit)
Cypress._.times(10, () => {
Cypress._.times(10, (i) => {
cy.visit("/projects/")
cy.get("a").contains('New project').click()
cy.get("a").contains('Create').first().click()
cy.get('input[name=name]').type(project_name)
cy.get('input[name=name]').type(`${project_name}-${i + 1}`);
cy.get("input[name=save]").contains('Create project').click()
});
cy.wait(5000) // sometimes it takes a while to create a project but just waiting once at the end should be enough
Expand Down Expand Up @@ -467,7 +467,7 @@ describe("Test project contributor user functionality", () => {
})

it("can create a file management instance", { defaultCommandTimeout: defaultCmdTimeoutMs }, () => {
const project_name = "e2e-create-proj-test"
const project_name = "e2e-create-proj-test-3"

cy.logf("Creating a blank project", Cypress.currentTest)
cy.createBlankProject(project_name)
Expand Down
1 change: 0 additions & 1 deletion portal/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ def get_unique_apps(queryset, app_ids_to_exclude):
app_orms = (app_model for app_model in APP_REGISTRY.iter_orm_models() if issubclass(app_model, SocialMixin))

for app_orm in app_orms:
logger.info("Processing: %s", app_orm)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason to remove this?

filters = ~Q(app_status__status="Deleted") & Q(access="public")
if collection:
filters &= Q(collections__slug=collection)
Expand Down
17 changes: 16 additions & 1 deletion templates/apps/create_base.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
{% load crispy_forms_tags %}

{% block content %}

{% load get_setting %}
{% include "breadcrumbs/bc_app_create.html" %}

<div class="row d-flex justify-content-center mt-2">
Expand Down Expand Up @@ -122,6 +122,19 @@ <h1 class="h3 mb-3 card-title">Create {{ form.model_name }}</h1>
}
}

// SS-1176 Users are able to set a custom default start URL for their apps
const subdomainInput = document.getElementById("id_subdomain");
const customDefaultUrlFormModifiedUrlText = document.getElementById("id_custom_default_url_prepend");

function updateUrlWithSubdomain() {
try {
const subdomainValue = subdomainInput.value.trim();
const displayValue = subdomainValue || "subdomain_name";
customDefaultUrlFormModifiedUrlText.innerHTML = `${displayValue}.{% get_setting 'DOMAIN' %}/</b>`;
} catch (error) {
console.log("No custom url subpath option in this app type.");
}
}

// User is "finished typing," do something
function doneTyping () {
Expand All @@ -141,6 +154,7 @@ <h1 class="h3 mb-3 card-title">Create {{ form.model_name }}</h1>
// OK because the input form and the server does the same.
checkSubdomainValidity(subdomain_input.toLowerCase());
}
updateUrlWithSubdomain();
}
function selectValidation(event) {
doneTyping(event.target.value);
Expand Down Expand Up @@ -183,6 +197,7 @@ <h1 class="h3 mb-3 card-title">Create {{ form.model_name }}</h1>
});
htmx.onLoad(function(content) {
runScript();
updateUrlWithSubdomain();
})
</script>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<div>
<div class="input-group">
{% if crispy_prepended_text %}
<span class="input-group-text">{{ crispy_prepended_text }}</span>
<span class="input-group-text fw-bold">{{ crispy_prepended_text }}</span>
{% endif %}
{{ field }}
{% if crispy_appended_text %}
Expand Down
Loading