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

fix: properly retrieve nested folder resources #1004

Merged
merged 1 commit into from
Jan 23, 2025

Conversation

dcshzj
Copy link
Contributor

@dcshzj dcshzj commented Jan 21, 2025

Problem

Nested folder resources are not properly retrieved, which causes the casing in the index page to not be correct for those cases.

Solution

Breaking Changes

  • Yes - this PR contains breaking changes
  • No - this PR is backwards compatible

Bug Fixes:

  • Correctly retrieve the folder resource using the full permalink instead.

Tests

@dcshzj dcshzj requested a review from a team as a code owner January 21, 2025 08:56
@datadog-opengovsg
Copy link

datadog-opengovsg bot commented Jan 21, 2025

Datadog Report

Branch report: fix/handle-nested-folders
Commit report: 371dd43
Test service: isomer-studio

✅ 0 Failed, 257 Passed, 36 Skipped, 49.76s Total Time
➡️ Test Sessions change in coverage: 1 no change

Copy link
Contributor

@seaerchin seaerchin left a comment

Choose a reason for hiding this comment

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

just for my understanding - previously this didn't work because we rely on fullPermalink === danglingDirectory, which meant that nested folders wouldn't work

now, since we use a full permalink comparison, the folder itself would match the permalink, even if nested?

@dcshzj
Copy link
Contributor Author

dcshzj commented Jan 22, 2025

@seaerchin yup, danglingDirectory is only the last part of the permalink (i.e. the slug, which is senior-management if the full permalink is about-us/senior-management). We can immediately tell this is true because the previous code does a replacement of dashes to spaces and capitalise the first letter to be the title, so means danglingDirectory is only the last part.

Now, it's doing a full comparison using the permalink (which we actually already are doing a few lines down, which I now moved up). This guarantees that we are getting the actual folder resource that we want, which we can then extract the title from.

@dcshzj dcshzj merged commit a72d1ad into main Jan 23, 2025
15 checks passed
@dcshzj dcshzj deleted the fix/handle-nested-folders branch January 23, 2025 01:08
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