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

Base templates: Apply DRY concept on base-templates #1914

Merged
merged 4 commits into from
Dec 16, 2024

Conversation

ebroda
Copy link
Contributor

@ebroda ebroda commented Dec 2, 2024

Summary of the discussion

There are five base templates which are quite similar.

  • base.html
  • base-full.html
  • base-profile.html
  • base-sidebar.html
  • base-wide-react.html

This Pull Request apply the DRY (Don't Repeat Yourself) concept, at least partly, on codes which have been identical in these files. Concrete these are the footer and navigation.

Additionally, links to github.com/openego/oeplatform have been updated to github.com/OpenEnergyPlatform/oeplatform.

Type of change (CHANGELOG.md)

Changes

  • Extract header/footer template (#1914)

Workflow checklist

Automation

PR-Assignee

Reviewer

  • 🐙 Follow the Reviewer Guidelines
  • 🐙 Provided feedback and show sufficient appreciation for the work done

@jh-RLI
Copy link
Contributor

jh-RLI commented Dec 4, 2024

Thank you! That was really needed. I've actually started doing some of this in #1896 as well (not merged yet), but I think I can easily use your approach. In the linked PR, we're working on replacing the OEP login system, which mainly uses a custom approach combined with django's built-in auth system. We will replace it with django allauth and also offer SSO with ORCID and other providers.

@ebroda
Copy link
Contributor Author

ebroda commented Dec 4, 2024

I think there's still some room for improvement / "Zusammenführung/Simplifizierung", as these files are still quite similar, mostly only differ by some classes which are somewhere set or not. But at least a start.

Regarding Auth: I build a variant for LDAP. If you want, I can also share that even though its a more specific case.

@jh-RLI
Copy link
Contributor

jh-RLI commented Dec 16, 2024

I agree ... there will be a short but funded project sometime next year just aimed at bug fixes/improvements. After that, I hope all these downside are finally fixed and the whole project is up to date using good practices. But I should take a look and just throw everything out of this “Wildwuchs” ;)

That's interesting! We will be using https://www.scc.kit.edu/dienste/regapp.php as there is a hosted service we can use. It is also a proxy for LDAP services. The documentation is a bit opaque. I'm not an expert in this area, but I think this covers our needs for now. LDAP could be useful if we want tighter integration with other services we maintain, right? Federated Login seems to be sufficient for now.

@jh-RLI jh-RLI merged commit 746fa1b into OpenEnergyPlatform:develop Dec 16, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants