Skip to content

Varkopat/enhancement/682 629 follow up prevent static hero fallback from bypassing status gating - #684

Open
Varkopat wants to merge 10 commits into
devfrom
Varkopat/enhancement/682-629-follow-up-prevent-static-hero-fallback-from-bypassing-status-gating
Open

Varkopat wants to merge 10 commits into
devfrom
Varkopat/enhancement/682-629-follow-up-prevent-static-hero-fallback-from-bypassing-status-gating

Conversation

@Varkopat

Copy link
Copy Markdown

📄 Pull Request Overview

Closes #682

🔧 Changes Made

  1. Successful empty Directus responses now remain empty, while static data is used only for actual request failures or unavailable Directus. Updated manager logic, hero detail routes, gallery/group views, navigation, and client detail loading.

  2. Added regression tests covering empty responses and failure fallback in heroApi.test.ts and HeroManager.test.ts.

I also did the following validations:

  • Focused Jest tests: 9 passed

  • git diff --check: passed


✅ Checklist Before Submission

  • Functionality: I have tested my code, and it works as expected.
  • JSDoc: I have added or updated JSDoc comments for all relevant code.
  • Debugging: No console.log() or other debugging statements are left.
  • Clean Code: Removed commented-out or unnecessary code.
  • Tests: Added new tests or updated existing ones for the changes made.
  • Documentation: Documentation has been updated (if applicable).

📝 Additional Information

Provide any additional context or information that reviewers may need to know:

  • Screenshot:

The defense gallery page doesn't show any heroes if they all are set as Draft/Archived on Directus:

screenshot

Successful empty Directus responses now remain empty, while static data is used only for actual request failures or unavailable Directus. Updated manager logic, hero detail routes, gallery/group views, navigation, and client detail loading.

Added regression tests covering empty responses and failure fallback in heroApi.test.ts and HeroManager.test.ts.

Validation:

- Focused Jest tests: 9 passed

- git diff --check: passed

- TypeScript check: only existing unrelated test errors remain.
@codecov-alt

codecov-alt Bot commented Sep 16, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 60.00000% with 18 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...-next-migration/src/entities/Hero/model/heroApi.ts 40.00% 9 Missing ⚠️
...rc/preparedPages/HeroesPages/ui/SingleHeroPage.tsx 0.00% 3 Missing ⚠️
...n/src/app/[lng]/(helper)/heroes/[slug]/_getPage.ts 0.00% 2 Missing ⚠️
...[lng]/(helper)/hero-development/[slug]/_getPage.ts 0.00% 1 Missing ⚠️
...on/src/entities/Hero/model/buildHeroQueryParams.ts 0.00% 1 Missing ⚠️
...ages/DefenseGalleryPages/ui/DefenseGalleryPage.tsx 0.00% 1 Missing ⚠️
...Pages/DefenseGalleryPages/ui/SingleDefensePage.tsx 0.00% 1 Missing ⚠️
Files with missing lines Coverage Δ
[...]/(helper)/defense-gallery/[herogroup]/_getPage.ts](https://yrfbcpxonsco.mikhail.com.de/gh/Alt-Org/Altzone-WebPages/pull/684?src=pr&el=tree&filepath=frontend-next-migration%2Fsrc%2Fapp%2F%5Blng%5D%2F%28helper%29%2Fdefense-gallery%2F%5Bherogroup%5D%2F_getPage.ts#diff-ZnJvbnRlbmQtbmV4dC1taWdyYXRpb24vc3JjL2FwcC9bbG5nXS8oaGVscGVyKS9kZWZlbnNlLWdhbGxlcnkvW2hlcm9ncm91cF0vX2dldFBhZ2UudHM=) 0.00% <ø> (ø)
...t-migration/src/entities/Hero/model/HeroManager.ts 62.96% <100.00%> (+62.96%) ⬆️
...on/src/entities/Hero/model/initializeHeroGroups.ts 100.00% <100.00%> (+100.00%) ⬆️
...eroGroups/ui/HeroGroupNavMenu/HeroGroupNavMenu.tsx 100.00% <100.00%> (+100.00%) ⬆️
...ation/src/widgets/SectionHeroesBlocks/ui/index.tsx 100.00% <100.00%> (+100.00%) ⬆️
...[lng]/(helper)/hero-development/[slug]/_getPage.ts 0.00% <0.00%> (ø)
...on/src/entities/Hero/model/buildHeroQueryParams.ts 0.00% <0.00%> (ø)
...ages/DefenseGalleryPages/ui/DefenseGalleryPage.tsx 0.00% <0.00%> (ø)
...Pages/DefenseGalleryPages/ui/SingleDefensePage.tsx 0.00% <0.00%> (ø)
...n/src/app/[lng]/(helper)/heroes/[slug]/_getPage.ts 0.00% <0.00%> (ø)
... and 2 more

... and 11 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@patinen patinen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice job overall. The page seems to be working now as expected.

One thing still needs fixing however:

frontend-next-migration/src/app/[lng]/(helper)/defense-gallery/[herogroup]/_getPage.ts still falls back to static hero data when Directus returns a successful but empty result:

if (Object.keys(groups).length === 0) {
groups = initializeHeroGroups(t);
}

This still bypasses the intended behavior, which menas a successful empty response is still treated as a reason to use static hero data on the server side. The returned groups are also used building the page metadata/SEO, including the group description etc. So static hero data can still leak into the response even though the visible page itself is empty.

Other than this, the follow-up logic looks good to me.

Fixed defense-gallery/[herogroup]/_getPage.ts.

Successful empty Directus responses now remain empty, including metadata/SEO generation. Static groups are used only when the Directus request throws.

Validation passed:

- Hero regression tests: 9 passed

- Diagnostics: no errors in the touched route

- git diff --check: passed
@Varkopat
Varkopat requested a review from patinen September 23, 2026 12:23

@patinen patinen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Awesome job! The remaining empty-response fallback has been removed, so successful empty Directus responses are no longer replaced with static data on the server side either.
Nice addition of the regression tests as well. Approved 👍

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.

629 Follow-up: Prevent static hero fallback from bypassing status gating

2 participants