Modular configurable message panel #197

jamesturner246 · 2024-11-01T16:57:44Z

Description

Refactors the messages partial for use with both process manager and controller. The Kafka topic of messages to be displayed can be set by using with when including the messages partial template -- e.g. {% include "main/messages.html" with topic="PROCMAN" %} -- using the topic key defined in settings.py.

Issues:

CSS is copied verbatim. Could probably do with being refactored out into dedicated CSS files.
Have not yet incorporated panel hiding etcetera to controller.

Fixes #162

Type of change

Documentation (non-breaking change that adds or improves the documentation)
New feature (non-breaking change which adds functionality)
Optimization (non-breaking, back-end change that speeds up the code)
Bug fix (non-breaking change which fixes an issue)
Breaking change (whatever its nature)

Key checklist

All tests pass (eg. python -m pytest)
The documentation builds and looks OK (eg. python -m sphinx -b html docs docs/build)
Pre-commit hooks run successfully (eg. pre-commit run --all-files)

Further checks

Code is commented, particularly in hard-to-understand areas
Tests added or an issue has been opened to tackle that in the future. (Indicate issue here: # (issue))

codecov-commenter · 2024-11-01T17:13:19Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.95%. Comparing base (0f5fc3e) to head (31377c7).
Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #197      +/-   ##
==========================================
+ Coverage   81.53%   81.95%   +0.41%     
==========================================
  Files          29       31       +2     
  Lines         390      399       +9     
==========================================
+ Hits          318      327       +9     
  Misses         72       72

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

cc-a

Looks good to me. Majority of the duplicated css in the controller index page can be removed but that's all..

cc-a · 2024-11-05T17:22:13Z

controller/templates/controller/index.html

@@ -2,3 +2,121 @@
 {% block title %}
  Controller
 {% endblock title %}
+{% block extra_css %}
+  <style>


Most of the things copied over don't do anything in this context. I've marked those that do, we can remove the rest.

All of this copied over CSS is what I mentioned in this comment on the PR that introduced it. We should probably find a better way to include it.

Opened #214.

controller/templates/controller/index.html

cc-a · 2024-11-05T17:24:52Z

drunc_ui/settings/settings.py

+    # ALL matches all topics.
+    "ALL": "^.*$",


Is this used anywhere?

It's used as the default value for topic in messages.html

cc-a · 2024-11-05T17:25:40Z

main/templates/main/messages.html

@@ -0,0 +1,30 @@
+{% load tz %}


Don't think this is needed.

There's a {% now "e (O)" as server_time %} that I think needs this.

I don't think now is one of the tags provided by tz - https://docs.djangoproject.com/en/5.1/topics/i18n/timezones/#template-tags

Nothing seems to break if I remove it. I'll take it out for now.

dalonsoa

LGTM!

I'll use this as the starting point for my issues on the controller.

dalonsoa · 2024-11-06T06:32:10Z

main/templates/main/messages.html

@@ -0,0 +1,30 @@
+{% load tz %}


There's a {% now "e (O)" as server_time %} that I think needs this.

AdrianDAlessandro

This looks good to me! I have some questions, but it seems to work (I can't fully confirm because I have no ERS messages in my feed)

AdrianDAlessandro · 2024-11-06T15:17:24Z

controller/templates/controller/index.html

@@ -2,3 +2,121 @@
 {% block title %}
  Controller
 {% endblock title %}
+{% block extra_css %}
+  <style>


All of this copied over CSS is what I mentioned in this comment on the PR that introduced it. We should probably find a better way to include it.

AdrianDAlessandro · 2024-11-06T15:28:41Z

drunc_ui/settings/settings.py

+    # ALL matches all topics.
+    "ALL": "^.*$",


It's used as the default value for topic in messages.html

tests/main/views/test_partial_views.py

process_manager/templates/process_manager/index.html

TinyMarsh

LGTM. Just a missing curly bracket is all.

controller/templates/controller/index.html

Sahil590

Code LGTM. I noticed that the decorator I added in #195 is not merged with this, so that would need to be done. MY current work also alters the partials file, so this would need to be updated before I can continue working on my issue.

jamesturner246 · 2024-11-07T13:13:43Z

@AdrianDAlessandro, I added a django command to populate arbitrary messages to arbitrary topics, if you want to have a check.

Run it using e.g.:

docker compose exec app python manage.py store_message -m "My test message." -t "erskafka-reporting"

jamesturner246 · 2024-11-07T14:12:15Z

This should be ready to merge whenever you wish to.

@Sahil590, I have moved your error wrapper into main/views/utils, and did the same with tests. This is because the message panel (uses the wrapper) is now a main partial. Looks like a big diff, but just file moving.

Sahil590

LGTM

jamesturner246 added 3 commits November 1, 2024 15:24

Refactor message pane into main:messages.

57ef636

Select message panel topic with django var.

90e8b37

Update tests.

291c2d5

jamesturner246 marked this pull request as ready for review November 1, 2024 17:12

jamesturner246 requested review from dalonsoa, cvzbynek, TinyMarsh, cc-a, alexdewar, AdrianDAlessandro and Sahil590 November 1, 2024 17:12

cc-a approved these changes Nov 5, 2024

View reviewed changes

dalonsoa approved these changes Nov 6, 2024

View reviewed changes

AdrianDAlessandro approved these changes Nov 6, 2024

View reviewed changes

TinyMarsh approved these changes Nov 6, 2024

View reviewed changes

controller/templates/controller/index.html Outdated Show resolved Hide resolved

Sahil590 reviewed Nov 6, 2024

View reviewed changes

Cleanup CSS.

ade397b

cvzbynek approved these changes Nov 7, 2024

View reviewed changes

jamesturner246 added 3 commits November 7, 2024 13:57

Merge branch 'main' into refactor_msg_pane

171ccbc

gitignore /docs/reference.

cc4b173

Fix view type.

d11037a

jamesturner246 requested review from Sahil590 and cc-a November 7, 2024 14:09

Remove {% load tz %}.

31377c7

Sahil590 approved these changes Nov 7, 2024

View reviewed changes

cc-a merged commit ee2a2eb into main Nov 7, 2024
4 checks passed

cc-a deleted the refactor_msg_pane branch November 7, 2024 16:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modular configurable message panel #197

Modular configurable message panel #197

jamesturner246 commented Nov 1, 2024 •

edited

Loading

codecov-commenter commented Nov 1, 2024 •

edited

Loading

cc-a left a comment

cc-a Nov 5, 2024

AdrianDAlessandro Nov 6, 2024

jamesturner246 Nov 7, 2024

cc-a Nov 5, 2024

AdrianDAlessandro Nov 6, 2024

cc-a Nov 5, 2024

dalonsoa Nov 6, 2024

cc-a Nov 7, 2024

jamesturner246 Nov 7, 2024

dalonsoa left a comment

dalonsoa Nov 6, 2024

AdrianDAlessandro left a comment

AdrianDAlessandro Nov 6, 2024

AdrianDAlessandro Nov 6, 2024

TinyMarsh left a comment

Sahil590 left a comment

jamesturner246 commented Nov 7, 2024 •

edited

Loading

jamesturner246 commented Nov 7, 2024

Sahil590 left a comment

Modular configurable message panel #197

Modular configurable message panel #197

Conversation

jamesturner246 commented Nov 1, 2024 • edited Loading

Description

Type of change

Key checklist

Further checks

codecov-commenter commented Nov 1, 2024 • edited Loading

Codecov Report

cc-a left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dalonsoa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AdrianDAlessandro left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TinyMarsh left a comment

Choose a reason for hiding this comment

Sahil590 left a comment

Choose a reason for hiding this comment

jamesturner246 commented Nov 7, 2024 • edited Loading

jamesturner246 commented Nov 7, 2024

Sahil590 left a comment

Choose a reason for hiding this comment

jamesturner246 commented Nov 1, 2024 •

edited

Loading

codecov-commenter commented Nov 1, 2024 •

edited

Loading

jamesturner246 commented Nov 7, 2024 •

edited

Loading