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.

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

First set of fixes to revive Django-based GUI code #695

Conversation

robertbartel
Copy link
Contributor

A set of changes to get older, Django-based GUI components for map and job configuration at least mostly loading and rendering properly, whereas before nothing or almost nothing would appear.

A few things in particular to point out:

  • Fix daphne dependency
  • Fix lack of automated running of Django migrations (broke startup for "new" GUI deployment)
  • Fix broken config for static files
  • Minimally fix GUI classes that were not up to date with dmod.communication and had required us to comment out some configured URLs

This is likely the first of several changes necessary to get things functional again.

@robertbartel robertbartel added bug Something isn't working maas MaaS Workstream labels Aug 2, 2024
@robertbartel robertbartel force-pushed the f/revise_django_gui/fix_broken_1 branch from ff349ef to 3044651 Compare August 6, 2024 14:56
<link rel="shortcut icon" type="image/png" href="{% static 'common/img/favicon.png' %}"/>
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.5.1/jquery.min.js"></script>
<link rel="stylesheet" href="{% static 'common/css/base.css' %}"/>
<link rel="stylesheet" href="{% static 'css/master.css' %}"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

be careful - this is mixing and matching resources. css/master.css is the stylesheet for the root level master.html template. common/css/base.css is the stylesheet for base.html.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think in this case that is what I wanted to do here, though I appreciate the cautious eye. I felt like we should start from the same styling throughout, so I was trying to move to that in this template base. That said, I could be talked into deferring on this if we think it's problematic to do it until it's also time to clean up some of the redundant files (which I was avoiding at the moment).

Copy link
Contributor

Choose a reason for hiding this comment

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

You might be better off changing to base template to master.html on the other templates rather than altering what css base.html is referencing. I haven't made the switch, so it might be fine, but both css files and templates are making assumptions based on what page elements are present.

You have the right idea, though. Switching what template you're inheriting from might work better here, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, if I'm remembering correctly, I started off by switching the base template, but moving from base.html caused some problem with pages' scripts. Swapping in the CSS change ended up being easier to follow along with and appeared to have fewer immediate side effects than carefully tweaking what seemed to be a good chunk of the Javascript setup (and hoping there wasn't something else needed after that).

Regardless, there isn't going to be a clean fix until we overhaul a good chunk of things. My goal at moment is to be minimalist and do just enough to get things working again, albeit with some attention paid to not laying any seriously traps for us to fall into later.

Marking several utility functions (used by some broken places in GUI
code) as deprecated, largely because they don't actually work.
Adding required STATIC_ROOT to be main static directory, and removing
the addition of this directory in things scanned by the static file
finders (i.e., since root, have other found things put here); this was
needed to fix issues with static files from main model exec GUI app,
which were not being found with previous config.
Adding daphne to dependencies.txt to install this requirment in GUI
image build.
Moving base maas config html template to use css/master.css instead of
common/css/base.css for consistent presentation.
Fixing some GUI classes that were broken and not update after changes to
dmod.communication package, and then re-added URL to Django config that
were taken out due to the previous classes breaking.
Hard-coding reasonable defaults for superuser setup values needed to get
previously existing migration to work.
Making sure to put it after lines that export the SQL_PASSWORD as
needed.
Copy link
Contributor

@christophertubbs christophertubbs left a comment

Choose a reason for hiding this comment

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

Good work - I look forward to seeing this evolve over time.

@christophertubbs christophertubbs merged commit 59d3f89 into NOAA-OWP:master Sep 6, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working maas MaaS Workstream
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants