Skip to content

Conversation

@zlayine
Copy link
Contributor

@zlayine zlayine commented Sep 12, 2025

PR Type

Bug fix


Description

  • Update package checks to core package

  • Fix sidebar feature flag detection


Diagram Walkthrough

flowchart LR
  Getters["Store getters: package presence checks"]
  Fuel["Fuel Tanks check -> platform-core"]
  Market["Marketplace check -> platform-core"]

  Getters -- "hasFuelTanksPackage" --> Fuel
  Getters -- "hasMarketplacePackage" --> Market
Loading

File Walkthrough

Relevant files
Bug fix
index.ts
Align package presence checks with core package                   

resources/js/store/index.ts

  • Replace platform-fuel-tanks check with platform-core
  • Replace platform-marketplace check with platform-core
  • Keep platform-beam check unchanged
+2/-2     

@github-actions
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Truthiness Bug

The getters return the result of Array.find, which is an object or undefined. If callers expect a boolean, this can cause subtle bugs. Consider returning a boolean with .some(...) or a double negation.

hasBeamPackage(state: AppState) {
    return state.config.packages.find((p) => p.name === 'platform-beam');
},
hasFuelTanksPackage(state: AppState) {
    return state.config.packages.find((p) => p.name === 'platform-core');
},
hasMarketplacePackage(state: AppState) {
    return state.config.packages.find((p) => p.name === 'platform-core');
},
Functional Change

Both hasFuelTanksPackage and hasMarketplacePackage now check for platform-core. Verify that consolidating to core is intended and that no feature flags rely on the specific packages.

hasFuelTanksPackage(state: AppState) {
    return state.config.packages.find((p) => p.name === 'platform-core');
},
hasMarketplacePackage(state: AppState) {
    return state.config.packages.find((p) => p.name === 'platform-core');
},

@github-actions
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Return a boolean presence check

Returning a found package object can be truthy for unrelated entries and leaks
structure to callers. Return a boolean with .some(...) to clearly signal presence
and avoid unexpected object usage.

resources/js/store/index.ts [254-256]

 hasFuelTanksPackage(state: AppState) {
-    return state.config.packages.find((p) => p.name === 'platform-core');
+    return state.config.packages.some((p) => p.name === 'platform-core');
 },
Suggestion importance[1-10]: 6

__

Why: Suggestion correctly targets the new getter that now returns a found object; switching to some yields a clear boolean and consistent API, improving maintainability without changing semantics.

Low
Use boolean return for getter

The getter likely intends a boolean, but .find(...) returns an object or undefined,
leading to inconsistent return types. Use .some(...) for a clear boolean contract.

resources/js/store/index.ts [257-259]

 hasMarketplacePackage(state: AppState) {
-    return state.config.packages.find((p) => p.name === 'platform-core');
+    return state.config.packages.some((p) => p.name === 'platform-core');
 },
Suggestion importance[1-10]: 6

__

Why: This mirrors the prior case: returning a boolean via some avoids leaking objects and ensures consistent getter typing, a reasonable quality improvement based on the new code.

Low

@zlayine zlayine merged commit 3872601 into master Sep 12, 2025
4 checks passed
@zlayine zlayine deleted the bugfix/PLA-2291/sidebar-links-fix branch September 12, 2025 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants