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

chore: settings cleanup & match Core-CMS #273

Merged
merged 12 commits into from
Feb 13, 2024

Conversation

wesleyboar
Copy link
Member

@wesleyboar wesleyboar commented Feb 13, 2024

Overview / Changes

  • edited settings headers to match Core-CMS
  • deleted unused, default, and inappropriate settings
  • edited & moved comments
  • deleted _ prefix from BRANDING settings
  • moved settings

Related

  • WI-110
    • this issue lists related PRs and commits from other repos

Testing

UI

Skipped.

Copy link
Member Author

@wesleyboar wesleyboar left a comment

Choose a reason for hiding this comment

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

Notes for reviewers.

Comment on lines -84 to -89

########################
# TACC: GOOGLE ANALYTICS
########################

GOOGLE_ANALYTICS_PROPERTY_ID = "G-5EQ8Y25ZTM"
Copy link
Member Author

Choose a reason for hiding this comment

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

Why? Cuz this is already in (and belongs in) local CMS settings file on TACC/Core-Portal-Deployments.

Comment on lines -5 to -13
########################
# DJANGO (EMAIL)
########################

# Set on server, NOT here
# https://tacc-main.atlassian.net/wiki/x/ZhVv
# EMAIL_BACKEND = "..."
# EMAIL_HOST = "..."
# DEFAULT_FROM_EMAIL = "..."
Copy link
Member Author

Choose a reason for hiding this comment

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

Why? Netsage does not send e-mail.

Comment on lines -15 to -28
########################
# DJANGO CMS SETTINGS
########################

CMS_TEMPLATES = (
('standard.html', 'Standard'),
('fullwidth.html', 'Full Width'),

('guide.html', 'Guide'),
('guides/getting_started.tam.html', 'Guide: Getting Started'),
('guides/data_transfer.html', 'Guide: Data Transfer'),
('guides/data_transfer.globus.html', 'Guide: Globus Data Transfer'),
('guides/portal_technology.html', 'Guide: Portal Technology Stack'),
)
Copy link
Member Author

Choose a reason for hiding this comment

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

Why? Netsage does not use any templates beyond the defaults (it even uses less).

Comment on lines -19 to +25
CMS_TEMPLATES = (
('standard.html', 'Standard'),
('fullwidth.html', 'Full Width'),
)
# CMS_TEMPLATES = (
# ('standard.html', 'Standard'),
# ('fullwidth.html', 'Full Width'),

# ('guide.html', 'Guide'),
# ('guides/portal_technology.html', 'Guide: Portal Technology Stack'),
# }
Copy link
Member Author

Choose a reason for hiding this comment

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

Why? In this example CMS, defaults are commented out to suggest you can change them. But defaults are not active, except for here. I am being consistent by commenting them out and showing the (current) defaults.

Comment on lines +14 to +20
########################
# TACC: SEARCH
########################

# Support Google search instead of Portal's Elasticsearch
SEARCH_PATH = '/site-search' # cuz Portal Nginx config hijacks /search
SEARCH_QUERY_PARAM_NAME = 'q' # as Google expects
Copy link
Member Author

Choose a reason for hiding this comment

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

You added a feature in this PR?! Nope. I moved the settings up from further below, to match Core-CMS oorder.

Comment on lines -15 to -20

('guide.html', 'Guide'),
('guides/getting_started.html', 'Guide: Getting Started'),
('guides/data_transfer.html', 'Guide: Data Transfer'),
('guides/data_transfer.globus.html', 'Guide: Globus Data Transfer'),
('guides/portal_technology.html', 'Guide: Portal Technology Stack')
Copy link
Member Author

Choose a reason for hiding this comment

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

Why? Most are deprecated. The rest are default.

Why not just use default i.e. delete CMS_TEMPLATES here? Cuz ECEP site has skilled (but not expert) editors, that maybe could receive Page Template privilege but would be confused by options they should not use.

Comment on lines 5 to -13
########################
# TACC: PORTAL
########################

# Does this CMS site have a portal (default value: True)?
INCLUDES_CORE_PORTAL = False
INCLUDES_PORTAL_NAV = False
INCLUDES_SEARCH_BAR = False
Copy link
Member Author

Choose a reason for hiding this comment

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

Woah, Pardner. Deleting these? Nope. Just moving them down further, to match Core-CMS order.

Comment on lines -35 to +23
TACC_BRANDING = [
"tacc",
"site_cms/img/org_logos/tacc-white.png",
"branding-tacc",
"https://www.tacc.utexas.edu/",
"_blank",
"TACC Logo",
"anonymous",
"True"
]

UTEXAS_BRANDING = [
"utexas",
"site_cms/img/org_logos/utaustin-white.png",
"branding-utaustin",
"https://www.utexas.edu/",
"_blank",
"University of Texas at Austin Logo",
"anonymous",
"True"
]

# NSF_BRANDING = [
# "nsf",
# "site_cms/img/org_logos/nsf-white.png",
# "branding-nsf",
# "https://www.nsf.gov/",
# "_blank",
# "NSF Logo",
# "anonymous",
# "True"
# ]

# BRANDING = [ TACC_BRANDING, UTEXAS_BRANDING, NSF_BRANDING ]
BRANDING = [ TACC_BRANDING, UTEXAS_BRANDING ] # this matches prod 2022
# BRANDING = [] # prod 2022 hides bar via snippet CSS but should also do this
BRANDING = False
Copy link
Member Author

Choose a reason for hiding this comment

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

What?!

  1. The active setting is BRANDING = [ TACC_BRANDING, UTEXAS_BRANDING ]... which is also the default. So delete it and all it's content/variables.
  2. The BRANDING is not desired (hence CSS hides it on prod site), so just set BRANDING = False to prevent template from showing branding bar.

@wesleyboar wesleyboar changed the title chore: settings cleanup & match Core-CMS and Core-Portal-Deployments chore: settings cleanup & match Core-CMS Feb 13, 2024
@wesleyboar
Copy link
Member Author

This is an almost non-functional change. The possible issues would be obvious (logo, favicon, branding) and caught in pre-prod. Merging this so WI-110 is complete and WI-111 can begin.

After WI-111, then I am confident to delete settings in repositories.

@wesleyboar wesleyboar merged commit a2109be into main Feb 13, 2024
@wesleyboar wesleyboar deleted the style/settings-clean-up-and-consistent-ify branch February 13, 2024 15:52
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.

1 participant