OpenWF/SpaceNinjaServer
12
6
Fork 15
Code Issues 46 Pull Requests 6 Actions Packages Projects Releases Activity

fix: tell client when it has used a free favor #850

Merged
Sainan merged 1 commits from fix-freefavorsused into main 2025-01-24 05:18:06 -08:00
Conversation 2 Commits 1 Files Changed 2 +16 -5
2 changed files with 16 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
src/services/purchaseService.ts
View File

@ -98,7 +98,14 @@ export const handlePurchase = async (
const inventory = await getInventory(accountId);
const affiliation = inventory.Affiliations.find(x => x.Tag == syndicateTag)!;
affiliation.FreeFavorsUsed ??= [];
affiliation.FreeFavorsUsed.push(affiliation.FreeFavorsEarned![affiliation.FreeFavorsUsed.length]);
const lastTitle = affiliation.FreeFavorsEarned![affiliation.FreeFavorsUsed.length];
affiliation.FreeFavorsUsed.push(lastTitle);
purchaseResponse.FreeFavorsUsed = [
{
Tag: syndicateTag,
Title: lastTitle
}
];
await inventory.save();
coderabbitai[bot] commented 2025-01-22 21:52:11 -08:00 (Migrated from github.com)
Review
Copy Link

⚠️ Potential issue

Add null checks and bounds validation to prevent runtime errors.

The current implementation has potential issues:

  1. No null check for FreeFavorsEarned before accessing it
  2. Array index access could be out of bounds
  3. No validation that FreeFavorsEarned has enough titles for the current favor usage

Consider this safer implementation:

-    const lastTitle = affiliation.FreeFavorsEarned![affiliation.FreeFavorsUsed.length];
-    affiliation.FreeFavorsUsed.push(lastTitle);
-    purchaseResponse.FreeFavorsUsed = [
-        {
-            Tag: syndicateTag,
-            Title: lastTitle
-        }
-    ];
+    if (!affiliation.FreeFavorsEarned?.length) {
+        throw new Error(`No free favors earned for syndicate: ${syndicateTag}`);
+    }
+    const lastTitleIndex = affiliation.FreeFavorsUsed.length;
+    if (lastTitleIndex >= affiliation.FreeFavorsEarned.length) {
+        throw new Error(`No more free favors available for syndicate: ${syndicateTag}`);
+    }
+    const lastTitle = affiliation.FreeFavorsEarned[lastTitleIndex];
+    affiliation.FreeFavorsUsed.push(lastTitle);
+    purchaseResponse.FreeFavorsUsed = [
+        {
+            Tag: syndicateTag,
+            Title: lastTitle
+        }
+    ];
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

                    if (!affiliation.FreeFavorsEarned?.length) {
                        throw new Error(`No free favors earned for syndicate: ${syndicateTag}`);
                    }
                    const lastTitleIndex = affiliation.FreeFavorsUsed.length;
                    if (lastTitleIndex >= affiliation.FreeFavorsEarned.length) {
                        throw new Error(`No more free favors available for syndicate: ${syndicateTag}`);
                    }
                    const lastTitle = affiliation.FreeFavorsEarned[lastTitleIndex];
                    affiliation.FreeFavorsUsed.push(lastTitle);
                    purchaseResponse.FreeFavorsUsed = [
                        {
                            Tag: syndicateTag,
                            Title: lastTitle
                        }
                    ];
_:warning: Potential issue_ **Add null checks and bounds validation to prevent runtime errors.** The current implementation has potential issues: 1. No null check for `FreeFavorsEarned` before accessing it 2. Array index access could be out of bounds 3. No validation that `FreeFavorsEarned` has enough titles for the current favor usage Consider this safer implementation: ```diff - const lastTitle = affiliation.FreeFavorsEarned![affiliation.FreeFavorsUsed.length]; - affiliation.FreeFavorsUsed.push(lastTitle); - purchaseResponse.FreeFavorsUsed = [ - { - Tag: syndicateTag, - Title: lastTitle - } - ]; + if (!affiliation.FreeFavorsEarned?.length) { + throw new Error(`No free favors earned for syndicate: ${syndicateTag}`); + } + const lastTitleIndex = affiliation.FreeFavorsUsed.length; + if (lastTitleIndex >= affiliation.FreeFavorsEarned.length) { + throw new Error(`No more free favors available for syndicate: ${syndicateTag}`); + } + const lastTitle = affiliation.FreeFavorsEarned[lastTitleIndex]; + affiliation.FreeFavorsUsed.push(lastTitle); + purchaseResponse.FreeFavorsUsed = [ + { + Tag: syndicateTag, + Title: lastTitle + } + ]; ``` <!-- suggestion_start --> <details> <summary>📝 Committable suggestion</summary> > ‼️ **IMPORTANT** > Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements. `````suggestion if (!affiliation.FreeFavorsEarned?.length) { throw new Error(`No free favors earned for syndicate: ${syndicateTag}`); } const lastTitleIndex = affiliation.FreeFavorsUsed.length; if (lastTitleIndex >= affiliation.FreeFavorsEarned.length) { throw new Error(`No more free favors available for syndicate: ${syndicateTag}`); } const lastTitle = affiliation.FreeFavorsEarned[lastTitleIndex]; affiliation.FreeFavorsUsed.push(lastTitle); purchaseResponse.FreeFavorsUsed = [ { Tag: syndicateTag, Title: lastTitle } ]; ````` </details> <!-- suggestion_end --> <!-- This is an auto-generated comment by CodeRabbit -->
} else {
const syndicate = ExportSyndicates[syndicateTag];

12
src/types/purchaseTypes.ts
View File

@ -34,12 +34,16 @@ export type IInventoryChanges = {
IBinChanges | number | object[] | IInfestedFoundryClient
>;
export interface IAffiliationMods {
Tag: string;
Standing?: number;
Title?: number;
}
export interface IPurchaseResponse {
InventoryChanges: IInventoryChanges;
Standing?: {
Tag: string;
Standing: number;
}[];
Standing?: IAffiliationMods[];
FreeFavorsUsed?: IAffiliationMods[];
BoosterPackItems?: string;
}