Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

added option to add additional items to the view-overview #183

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

added option to add additional items to the view-overview #183

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 8 additions & 3 deletions pyscada/hmi/templates/view_overview.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,21 @@
<div class="row">
{% for view in view_list %}
{% if view.visible %}
{% if view.link_title %}
{% url 'main-view' view.link_title as link_url %}
{% else %}
{% url 'view-overview-link' view.link_url as link_url %}
{% endif %}
<div class="col-sm-3">
<div class="panel panel-default">
<div class="panel-heading">
<h3 class="panel-title"><a href="{% url 'main-view' view.link_title %}" target="{{ link_target }}" >{{ view.title }}</a></h3>
<h3 class="panel-title"><a href="{{ link_url }}" target="{{ link_target }}" >{{ view.title }}</a></h3>
</div>
<div class="panel-body">
{% if view.logo %}
<a href="{% url 'main-view' view.link_title %}" class="thumbnail" target="{{ link_target }}" ><img src="{{ view.logo.url }}" alt="{{ view.description }}" width="100%" ></a>
<a href="{{ link_url }}" class="thumbnail" target="{{ link_target }}" ><img src="{{ view.logo.url }}" alt="{{ view.description }}" width="100%" ></a>
{% else %}
<a href="{% url 'main-view' view.link_title %}" class="thumbnail" target="{{ link_target }}" ><p style="min-height: 120px;">{{ view.description }}</p></a>
<a href="{{ link_url }}" class="thumbnail" target="{{ link_target }}" ><p style="min-height: 120px;">{{ view.description }}</p></a>
{% endif %}
</div>
</div>
Expand Down
3 changes: 3 additions & 0 deletions pyscada/hmi/urls.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,7 @@
views.get_hidden_config2,
name="get-hidden-config2",
),
path(
"<link_title>/", views.index, name="view-overview-link"
), # this is used to add special links to the view-overview
]
50 changes: 47 additions & 3 deletions pyscada/hmi/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,60 @@

logger = logging.getLogger(__name__)

class ViewDummy:
def __init__(
self,
title="",
link_title=None,
link_url="",
description="",
url="",
visible=True,
):
self.title = title
self.link_title = link_title
self.link_url = link_url
self.description = description

class Logo:
def __init__(self, url):
self.url = url

def __eq__(self, obj):
return obj == self.url

def __hash__(self):
return hash(self.url)

def __bool__(self):
return bool(self.url)

self.logo = Logo(url=url)
self.visible = visible

@login_required
def index(request):
def index(request, link_title=""):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why did you add this ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, I understood, for a dummy view defined in the settings.py :

  • If link_title is set, it redirect to a view (why let this option if this is the classic way to add a view from the admin ?)
  • If link_title is not set, it redirect to a local link on the server but you define an URL to catch it (and redirect to the index view function), so it will not redirect to your other plugin or django app if the those urls.py is loaded after the hmi/urls.py
  • It is not possible to add an external link (ex: https://pyscada.readthedocs.io/)
  • It could be better to manage this in the admin, because changing the settings.py needs to restart gunicorn.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If link_title is not set, it redirect to a local link on the server but you define an URL to catch it (and redirect to the index view function), so it will not redirect to your other plugin or django app if the those urls.py is loaded after the hmi/urls.py

I'm not sure what you mean with this, at least in my case (and I admit, that I didn't think to much about other cases were one would like to add links to the view-overview) this is not a problem, but i see that there is a chance that the user may set a custom link with this new way which overrides a internal url of pyscada itself or a pyscada-plugin

It is not possible to add an external link (ex: https://pyscada.readthedocs.io/)

this is True, to have this option more changes to the view-overview template will be necessary, for my case where I use nginx as reverse proxy for the other apps, this is not a problem, but i see your point.

It could be better to manage this in the admin, because changing the settings.py needs to restart gunicorn.

In my opinion the need to restart gunicorn is not a problem, my intention was to use it not dynamically, but if we decide later to have it as dedicated model then it would be possible to keep more or less the same logic.

When I implemented this I was not sure what others may have build on top of the view-overview template and therefore tried to keep the changes to that minimal. I agree with you, that if we want to add the ability to add links more freely (e.g. external links) or allow dynamic changes, this would be not the right way. But for links in the same Namespace as Pyscada (same Top Level URL) hijacking the index Funktion in views is the only good option I see. There would be the other option to have a special URL for that (e.g. https://TOP_LEVEL_URL/external/NEW_NAMESPACE) that I don't like so much because this would make those urls harder to memories.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure what you mean with this, at least in my case (and I admit, that I didn't think to much about other cases were one would like to add links to the view-overview) this is not a problem, but i see that there is a chance that the user may set a custom link with this new way which overrides a internal url of pyscada itself or a pyscada-plugin

For example, this config add a view that redirect to https://127.0.0.1/test/.

OVERVIEW_ADDITIONAL_LINKS = [
{
"title": "test",
"link_title": False,
"link_url": "test",
"description": "desc",
}
]

As this url is not defined in nginx as a location and no plugin is catching this path before the hmi.urls.py, the path you defined in hmi.urls.py redirect this link to view-overview with the link https://127.0.0.1/test/ (the index view).

########

this is True, to have this option more changes to the view-overview template will be necessary, for my case where I use nginx as reverse proxy for the other apps, this is not a problem, but i see your point.

This should do the trick :

        {% if view.visible %}
            {% with link_url=view.link_url %}
            {% if view.link_title %}
                {% url 'main-view' view.link_title as link_url %}
            {% elif "http" not in link_url %}
                {% url 'view-overview-link' view.link_url as link_url %}
            {% endif %}
        <div class="col-sm-3">
        <div class="panel panel-default">
                <div class="panel-heading">
                    <h3 class="panel-title"><a href="{{ link_url }}" target="{{ link_target }}" >{{ view.title }}</a></h3>
                </div>
                <div class="panel-body">
                {% if view.logo %}
                <a href="{{ link_url }}" class="thumbnail" target="{{ link_target }}" ><img src="{{ view.logo.url }}" alt="{{ view.description }}"  width="100%" ></a>
                {% else %}
                <a href="{{ link_url }}" class="thumbnail" target="{{ link_target }}" ><p style="min-height: 120px;">{{ view.description }}</p></a>
                {% endif %}
            </div>
           </div>
        </div>
            {% endwith %}
        {% endif %}

########

In my opinion the need to restart gunicorn is not a problem, my intention was to use it not dynamically, but if we decide later to have it as dedicated model then it would be possible to keep more or less the same logic.

When I implemented this I was not sure what others may have build on top of the view-overview template and therefore tried to keep the changes to that minimal. I agree with you, that if we want to add the ability to add links more freely (e.g. external links) or allow dynamic changes, this would be not the right way. But for links in the same Namespace as Pyscada (same Top Level URL) hijacking the index Funktion in views is the only good option I see. There would be the other option to have a special URL for that (e.g. https://TOP_LEVEL_URL/external/NEW_NAMESPACE) that I don't like so much because this would make those urls harder to memories.

I think it should be easy from the admin to:

  • add link in the view-overview (internal or external)
  • choose what is the default page after login (view-overview or a choosen view) for a specific group
  • give access to some views, view-overview, widgets without login
  • define various view-overview
  • be able to change the template used for a view or for the view-overview (using a theme mechanism, as I tried to implement for the views)
  • add a theme with options (color, logo...) with a plugin (what I did for the views, but I think it may be done better)

What do you think ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

as a first step I will try your code Suggestion for the view-overview template

if GroupDisplayPermission.objects.count() == 0:
view_list = View.objects.all()
else:
view_list = get_group_display_permission_list(
View.objects, request.user.groups.all()
view_list = list(
get_group_display_permission_list(View.objects, request.user.groups.all())
)

if hasattr(settings, "OVERVIEW_ADDITIONAL_LINKS"):
for view_data in settings.OVERVIEW_ADDITIONAL_LINKS:
view = ViewDummy()
view.logo.url = view_data["logo_url"] if "logo_url" in view_data else None
view.title = view_data["title"] if "title" in view_data else ""
view.link_title = (
view_data["link_title"] if "link_title" in view_data else None
)
view.link_url = view_data["link_url"] if "link_url" in view_data else ""
view.description = (
view_data["description"] if "description" in view_data else ""
)
view_list.append(view)

c = {
"user": request.user,
"view_list": view_list,
Expand Down