feat(client): discoverable apps notification fixed

[shubham-sp-07]

Description

The user does not have access to view this project or project id is invalid notification msg fixed for discoverable page

Changes Made

How to Test

1. Steps to reproduce/test the behavior
2. Expected outcomes

Notes

[github-actions]

@CodiumAI-Agent /describe

[CodiumAI-Agent]

Title

feat(client): discoverable apps notification fixed

---

User description

Description

The user does not have access to view this project or project id is invalid notification msg fixed for discoverable page

Changes Made

How to Test

1. Steps to reproduce/test the behavior
2. Expected outcomes

Notes

---

PR Type

Bug fix

---

Description

• Await permission before fetching app data

• Dynamically include dependencies fetch

• Skip dependencies for discoverable users

• Refactor Promise.allSettled usage

---

Changes walkthrough 📝

	Relevant files
Bug fix	AppDetailPage.tsx` Conditional dependencies fetching logic                                    packages/client/src/pages/app/AppDetailPage.tsx Added await getPermission() before data fetch Built promises array dynamically Conditionally pushed fetchDependencies Handled results with Promise.allSettled
Additional files	AppDetailPage.tsx +8/-3

---

Need help?

Type /help how to ... in the comments thread for any questions about PR-Agent usage.

Check out the documentation for more information.

[github-actions]

@CodiumAI-Agent /review

[CodiumAI-Agent]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Error handling in getPermission The call to getPermission() is awaited but lacks try/catch; if it rejects, subsequent code using permission may behave unexpectedly. Consider adding error handling or fallback logic. await getPermission(); const permission = getValues('permission'); Permission value retrieval After awaiting getPermission(), getValues('permission') may not immediately reflect the updated permission. Verify that getPermission correctly sets the form value before use. await getPermission(); const permission = getValues('permission'); const promises: Promise<any>[] = [ Promise pattern consistency Mixed use of async/await and Promise.allSettled.then may complicate error flows. Consider unifying to async/await with try/catch for clarity. fetchAppInfo( monolithStore, id, configStore.store.config.projectMetaKeys.map((a) => a.metakey), ), fetchMainUses(monolithStore, id), ]; if (permission !== 'discoverable') { promises.push(fetchDependencies(monolithStore, id)); } Promise.allSettled(promises).then((results) =>

[github-actions]

@CodiumAI-Agent /improve

[CodiumAI-Agent]

PR Code Suggestions ✨

Latest suggestions up to d7d9af4

Category	Suggestion	Impact
Possible issue	Handle permission fetch errors Wrap the getPermission call in a try/catch to catch and surface any errors, preventing uncaught exceptions from stopping execution. If it fails, emit a message and abort further processing. packages/client/src/pages/app/AppDetailPage.tsx [242-243] -await getPermission(); +try { + await getPermission(); +} catch (err) { + emitMessage(true, err); + return; +} const permission = getValues('permission'); Suggestion importance[1-10]: 7 __ Why: Wrapping getPermission() in a try/catch ensures any errors are caught and reported via emitMessage, preventing uncaught exceptions from breaking fetchAppData.	Medium
General	Batch state updates Batch the two separate setValues calls into a single update to avoid redundant state updates and potential field overrides. This reduces re-renders and ensures the merged object contains both markdown and all parsedMeta fields in one go. packages/client/src/pages/app/AppDetailPage.tsx [305-309] setValues((prev) => ({ ...prev, markdown: parsedMeta.markdown || '', + ...parsedMeta, })); -setValues((prev) => ({ ...prev, ...parsedMeta })); Suggestion importance[1-10]: 6 __ Why: Batching the two setValues calls reduces redundant state updates and extra re-renders while still merging markdown and all parsedMeta fields correctly.	Low
	Destructure settled results Deconstruct the settled results array into named variables rather than using numeric indices, which improves clarity and reduces the risk of misalignment if the array order changes. packages/client/src/pages/app/AppDetailPage.tsx [244-254] -const promises: Promise<any>[] = [ +const promises = [ fetchAppInfo(monolithStore, id, configStore.store.config.projectMetaKeys.map((a) => a.metakey)), fetchMainUses(monolithStore, id), + ...(permission !== 'discoverable' ? [fetchDependencies(monolithStore, id)] : []), ]; -if (permission !== 'discoverable') { - promises.push(fetchDependencies(monolithStore, id)); -} +const results = await Promise.allSettled(promises); +const [appInfoResult, mainUsesResult, dependenciesResult] = results; Suggestion importance[1-10]: 5 __ Why: Destructuring the results array into named variables improves readability and reduces reliance on numeric indices without altering the existing logic.	Low
	Validate permission value Validate that the retrieved permission is one of the expected values before using it. This guards against typos or unexpected form states. packages/client/src/pages/app/AppDetailPage.tsx [243] const permission = getValues('permission'); +const validPermissions = ['author', 'editor', 'readOnly', 'discoverable'] as const; +if (!validPermissions.includes(permission as typeof validPermissions[number])) { + console.warn(`Unexpected permission: ${permission}`); +} Suggestion importance[1-10]: 4 __ Why: Adding a check that permission is one of the expected values guards against typos or unexpected form states but doesn’t address a critical bug.	Low

---

Previous suggestions

Suggestions up to commit d7d9af4

Category	Suggestion	Impact
Possible issue	Guard against undefined display_options Add a guard to ensure found is not undefined before accessing display_options, preventing potential runtime errors when a key is missing. packages/client/src/pages/app/AppDetailPage.tsx [274-289] const found = projectMetaKeys.find( (obj) => obj.metakey === curr, ); … } else if ( - found.display_options === 'single-typeahead' || - found.display_options === 'select-box' || - found.display_options === 'multi-typeahead' + found?.display_options === 'single-typeahead' || + found?.display_options === 'select-box' || + found?.display_options === 'multi-typeahead' ) { Suggestion importance[1-10]: 7 __ Why: The code accesses found.display_options without checking for found, risking a runtime error when no matching key is found.	Medium
	Handle getPermission errors Wrap the getPermission call in a try/catch to handle rejections and emit an error message instead of letting unhandled exceptions bubble up. packages/client/src/pages/app/AppDetailPage.tsx [242-243] -await getPermission(); +try { + await getPermission(); +} catch (err) { + emitMessage(true, err); + return; +} const permission = getValues('permission'); Suggestion importance[1-10]: 6 __ Why: Wrapping getPermission in a try/catch prevents unhandled promise rejections and ensures errors are emitted via emitMessage.	Low
General	Batch update form values Combine the two setValues calls into one to batch update the form state and avoid intermediate state inconsistencies. packages/client/src/pages/app/AppDetailPage.tsx [305-309] setValues((prev) => ({ ...prev, markdown: parsedMeta.markdown || '', + ...parsedMeta, })); -setValues((prev) => ({ ...prev, ...parsedMeta })); Suggestion importance[1-10]: 5 __ Why: Combining the two setValues calls reduces redundant re-renders and avoids intermediate state, improving maintainability.	Low
	Clear dependencies when discoverable Explicitly clear dependencies and selectedDependencies when permission is 'discoverable' to avoid stale data from previous loads. packages/client/src/pages/app/AppDetailPage.tsx [252-254] if (permission !== 'discoverable') { promises.push(fetchDependencies(monolithStore, id)); +} else { + setValue('dependencies', []); + setValue('selectedDependencies', []); } Suggestion importance[1-10]: 5 __ Why: Explicitly clearing dependencies when skipping the fetch prevents stale data from previous loads under 'discoverable' permission.	Low

Suggestions up to commit 0b45cd2

Category	Suggestion	Impact
Possible issue	Await Promise settlement Replace the .then-based handling of Promise.allSettled with an await so that fetchAppData truly pauses until all fetch operations complete, ensuring errors are handled in sequence. packages/client/src/pages/app/AppDetailPage.tsx [255-259] -Promise.allSettled(promises).then((results) => - results.forEach((res, idx) => { - if (res.status === 'rejected') { - emitMessage(true, res.reason); - } else { - // ... - } - }) -); +const results = await Promise.allSettled(promises); +results.forEach((res, idx) => { + if (res.status === 'rejected') { + emitMessage(true, res.reason); + } else { + // ... + } +}); Suggestion importance[1-10]: 7 __ Why: Replacing the un-awaited .then with await Promise.allSettled ensures fetchAppData waits for all operations to complete, fixing potential async timing bugs.	Medium
General	Use direct permission return Have getPermission return the fetched permission directly and assign it to permission instead of reading from form state afterward, avoiding potential stale values. packages/client/src/pages/app/AppDetailPage.tsx [242-243] -await getPermission(); -const permission = getValues('permission'); +const permission = await getPermission(); Suggestion importance[1-10]: 5 __ Why: Returning the permission directly from getPermission avoids stale form state reads, improving reliability though it's a minor API change.	Low

[shubham-sp-07]

@CodiumAI-Agent /improve

[shubham-sp-07]

Please Refer first commit for issue fix

[AAfghahi]

see comments

[AAfghahi]

is the array always the same way? Its never structured differently? One thing that worries me about this if else statement is that if the structure ever changes it would break.

Is there another check that we can make that is a bit more stringent?

[anurag91jain]

@AAfghahi the indexes are based on the array insertion in the promises. It will always remain the same till the array is changed

[AAfghahi]

Could we add error checking to this as well? Does not look like we do anything if the pixel call errors out.

[anurag91jain]

Individual calls are tracked within the indexes and if the overall call fails, that is taken as rejected.

[github-actions]

@CodiumAI-Agent /update_changelog