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

9 interview card data #24

Merged
merged 5 commits into from
May 8, 2024
Merged

9 interview card data #24

merged 5 commits into from
May 8, 2024

Conversation

jtmst
Copy link
Contributor

@jtmst jtmst commented Apr 29, 2024

This PR addresses issue 19 as well as issue 21

Changes:

  • Splits TopicsCard into its own component file with updated logic for displaying interviews
  • Adds fetchInterviewData file.
  • fetchInterviews logic. Sorts interviews by topic.
  • Simplify page.tsx. Adds filter and sort logic for topic cards so that only topics with associated interviews are rendered (issue 21)
  • Modify topic config data so all always_visible flags are set to false
  • Modify server config file for easier addition of jurisdiction data later
  • File change count inflated because of yarn cache refresh
  • File organization changes, (config files in config dir)
image

@jtmst jtmst marked this pull request as ready for review April 29, 2024 18:21
@johnphamvan
Copy link

@nonprofittechy We acknowledge that the .yarn/cache makes these PRs very unwieldy. That's the current yarn best practice right now though. See https://github.com/yarnpkg/berry?tab=readme-ov-file#building-your-own-bundle. We recommend unchecking zip files from the file list.

image

@nonprofittechy
Copy link
Member

No worries about the .zip files. I had some trouble running this branch locally but I'll come back with a more specific question.

@jtmst
Copy link
Contributor Author

jtmst commented May 1, 2024

No worries about the .zip files. I had some trouble running this branch locally but I'll come back with a more specific question.

Anything I can help out with getting it to run locally?

@nonprofittechy
Copy link
Member

No worries about the .zip files. I had some trouble running this branch locally but I'll come back with a more specific question.

Anything I can help out with getting it to run properly?

So I did both a yarn install and then yarn dev, as well as npm install and npm run start but I just get a 404 error.

Maybe I'm having trouble understanding what the new routes are?

@jtmst
Copy link
Contributor Author

jtmst commented May 1, 2024

No worries about the .zip files. I had some trouble running this branch locally but I'll come back with a more specific question.

Anything I can help out with getting it to run properly?

So I did both a yarn install and then yarn dev, as well as npm install and npm run start but I just get a 404 error.

Maybe I'm having trouble understanding what the new routes are?

Routes should be unchanged and you should still see the homepage on localhost:3000/

@nonprofittechy
Copy link
Member

This what I get. Note that I can run other node applications from the same install (and I could run this in the past) so it's not a firewall issue:

image

Comment on lines +22 to +27
.filter(
(topic) =>
topic.always_visible ||
(interviewsByTopic[topic.name] &&
interviewsByTopic[topic.name].length > 0)
)
Copy link
Member

Choose a reason for hiding this comment

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

LIST is a hierarchical code. 209A, which is FA-07-00-00-00, should go into the "Family" topic but it's getting sorted into Other

image

image

Copy link
Member

@nonprofittechy nonprofittechy left a comment

Choose a reason for hiding this comment

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

This is looking good except we need to parse the LIST topics.

Edit: I suggest a function to see if a topic is contained in or is identical to a top level topic. The current site has some lower-level topics that we promoted to the "top" level, such as Appeals and Domestic Violence. So it's not just enough to strip off all of the numbers.

@jtmst jtmst requested a review from nonprofittechy May 8, 2024 19:36
@jtmst jtmst merged commit 282ed98 into main May 8, 2024
2 checks passed
@samglover samglover deleted the 9-interview-card-data branch September 20, 2024 03:19
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.

3 participants