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

Cms unifying filename id 188347468 #9352

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

adidner
Copy link
Contributor

@adidner adidner commented Jan 3, 2025

Description

we wanted to unify filename and id into just id across the CMS. This is because we've had some bugs over time that are a result of this and we want a single source of truth moving forward.

Ticket

This pull request resolves #188347468 and #188671780.

Approach

we used vscode global search and regex's to find and replace filename with id where relevant or delete filename where both existed. Then we used a script to double check that all id's matched actual incode filenames.

Also things like label and displayname that referenced filename now reference id

we also changed all our dropdowns in the config.yml file to reference ID instead.

Steps to Test

look through CMS and make sure things look alright.
Poke around the app to make sure the various types of things aren't broken (look for a license, look for a generic task, look for some roadmap variations etc)

Notes

Do we want to keep "archived" collections? - idk but its a sand, because I don't want to talk to content more during this ticket, will slow things down unnecessarily. So I'mma fix it for now real quick

in loadAllFilings and loadAllFilingsbyUrlSlug we have some wonky pattern stuff going on where we need both loadAllFilingsByUrlSlug and loadAllFilingsByFilename but really we probably shouldn't and it can be made smoother

Code author checklist

  • I have rebased this branch from the latest main branch
  • I have performed a self-review of my code
  • I have created and/or updated relevant documentation on the engineering documentation website
  • I have not used any relative imports
  • I have pruned any instances of unused code
  • I have not added any markdown to labels, titles and button text in config
  • If I added/updated any values in userData (including profileData, formationData etc), then I added a new migration file
  • I have checked for and removed instances of unused config from CMS
  • If I added any new collections to the CMS config, then I updated the search tool and cmsCollections.ts (see CMS Additions in Engineering Reference/FAQ on the engineering documentation site)
  • I have updated relevant .env values in both .env-template and in Bitwarden

@@ -4,25 +4,25 @@ summaryDescriptionMd: |-

A few sentences with the most important information is enough.
urlSlug: task-test
filename: task-test
Copy link
Contributor

Choose a reason for hiding this comment

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

@adidner we need to delete this

@@ -1332,12 +1320,13 @@ collections:
- { label: "Disabled Veteran Owned", value: "disabled-veteran" }
- { label: "LGBTQ Owned", value: "lgbtq-owned" }
- { label: "is SBE (show if fewer than 120 employees)", name: "isSbe", widget: boolean, default: false }
# TODO: Do we want to keep "archived" collections? - idk but its a sand, because I don't want to talk to content more during this ticket, will slow things down unnecessarily. So I'mma fix it for now real quick
Copy link
Contributor

Choose a reason for hiding this comment

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

@adidner can you add information about this to the PR description, please?

@@ -2482,12 +2453,12 @@ collections:
required: false,
}

- name: "tasks"
- name: "tasks" # TODO: also some webflow stuff and sync script stuff to do
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- name: "tasks" # TODO: also some webflow stuff and sync script stuff to do
- name: "tasks"

@@ -2693,20 +2632,20 @@ collections:
- { label: "Call To Action Text", name: "callToActionText", widget: "string", required: false }
- { label: "Call To Action Link", name: "callToActionLink", widget: "string", required: false }

- name: "webflow-licenses"
- name: "webflow-licenses" # TODO: fuck with sync scripts, also there is no ID here soooo, are we replacing filename with ID?
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- name: "webflow-licenses" # TODO: fuck with sync scripts, also there is no ID here soooo, are we replacing filename with ID?
- name: "webflow-licenses"

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