tethysplatform / tethys

The Tethys Platform main Django website project repository.
http://tethysplatform.org/
BSD 2-Clause "Simplified" License
94 stars 51 forks source link

Avoid using the app package as a hardcoded string #1029

Closed sdc50 closed 7 months ago

sdc50 commented 8 months ago

Fixes #997

Compiles all Tethys template tags/filters into a single tethys namespace templatetag library.

Add template filters and TethysBase classmethods to use inplace of django.shortcuts functions to avoid using the app.package as a magic string

Add the following template filters:

{% load static tethys %}

{% static tethys_app|public:'css/main.css' %}
{% load tethys %}

{% url tethys_app|url:'home' %}

Changes the scaffold to extend the base.html without using the project:

{% extends tethys_app.package|add:"/base.html" %}

Changes scaffolds to name app/ext classes as App and Extension.

Adds render, redirect, reverse, and render_to_string methods to tethys_apps.base.app_base.TethysBase to replace the django.shortcuts functions:

from tethys_sdk.gizmos import Button
from .app import App

@controller
def home(request):
    """
    Controller for the app home page.
    """

    cancel_button = Button(
        display_text='Cancel',
        name='cancel-button',
        href=app.reverse('home')
    )

    context = {'cancel_button': cancel_button}

    return App.render(request, 'home.html', context)
from .app import App

@controller
def do_something_and_redirect_home(request):
    """
    Controller to do something before redirecting to the app homepage.
    """

    return App.redirect(App.reverse("jobs_table"))
coveralls commented 7 months ago

Coverage Status

coverage: 100.0%. remained the same when pulling 52c904fd995a2b082f029154308faf9c3607487c on avoid_hardcoded_app_package into cf6ebe6099663db3de683d94a04fefa6508e286d on main.

sdc50 commented 7 months ago

Change the scaffolds so the class name of the TethysAppBase and TethysExtensionBase objects that are generated have a generic name.

sdc50 commented 7 months ago

@swainn I still have no idea why the test_update_row_exception test was causing an error on the CI. I couldn't get it to error locally. The error was due to it not being able to find the tethys template tag library when rendering the job_row_error.html template. As it turns out, that template doesn't actually use any tethys template filters, so I just remove it from the load tag. That's allowing tests to pass, but I'm stiff baffled as to what the issue was.

Let me know what you think about Ext vs. Extension and my justification for importing Extension as a custom name in the example.

swainn commented 7 months ago

@swainn I still have no idea why the test_update_row_exception test was causing an error on the CI. I couldn't get it to error locally. The error was due to it not being able to find the tethys template tag library when rendering the job_row_error.html template. As it turns out, that template doesn't actually use any tethys template filters, so I just remove it from the load tag. That's allowing tests to pass, but I'm stiff baffled as to what the issue was.

Let me know what you think about Ext vs. Extension and my justification for importing Extension as a custom name in the example.

I have an idea of what it might be. I didn't see any changes in the tethys_sdk module in the PR:

image

Did you forget to commit it?

Also, I don't think tethys_sdk is a Django app, so I'm not sure how it is loading template tags from there.

sdc50 commented 7 months ago

I have an idea of what it might be. I didn't see any changes in the tethys_sdk module in the PR:

You are absolutely correct! 🤦‍♂️

Also, I don't think tethys_sdk is a Django app, so I'm not sure how it is loading template tags from there.

tethys_sdk is a Django app and it's listed in INSTALLED_APPS:

https://github.com/tethysplatform/tethys/blob/main/tethys_portal/settings.py#L232