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

feat: changing equipped shawzin/instrument #762

Merged
Sainan merged 1 commits from change-shawzin into main 2025-01-11 03:54:33 -08:00
Conversation 2 Commits 1 Files Changed 3 +20 -1
3 changed files with 20 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
src/controllers/api/setEquippedInstrumentController.ts Normal file
View File

@ -0,0 +1,17 @@
import { RequestHandler } from "express";
import { getAccountIdForRequest } from "@/src/services/loginService";
import { getInventory } from "@/src/services/inventoryService";
import { getJSONfromString } from "@/src/helpers/stringHelpers";
export const setEquippedInstrumentController: RequestHandler = async (req, res) => {
const accountId = await getAccountIdForRequest(req);
const inventory = await getInventory(accountId);
const body = getJSONfromString(String(req.body)) as ISetEquippedInstrumentRequest;
inventory.EquippedInstrument = body.Instrument;
await inventory.save();
res.end();
};
coderabbitai[bot] commented 2025-01-10 23:14:30 -08:00 (Migrated from github.com)
Review
Copy Link

🛠️ Refactor suggestion

Add error handling and input validation.

The controller needs to handle potential errors and validate input:

  1. Add try-catch block for error handling
  2. Validate the Instrument string
  3. Set appropriate response status codes

Apply this diff to improve error handling and validation:

 export const setEquippedInstrumentController: RequestHandler = async (req, res) => {
+    try {
+        if (!req.body) {
+            return res.status(400).json({ error: 'Request body is required' });
+        }
         const accountId = await getAccountIdForRequest(req);
+        if (!accountId) {
+            return res.status(401).json({ error: 'Unauthorized' });
+        }
         const inventory = await getInventory(accountId);
         const body = getJSONfromString(String(req.body)) as ISetEquippedInstrumentRequest;
+        if (!body.Instrument || typeof body.Instrument !== 'string') {
+            return res.status(400).json({ error: 'Invalid instrument value' });
+        }
         inventory.EquippedInstrument = body.Instrument;
         await inventory.save();
-        res.end();
+        res.status(200).json({ message: 'Instrument equipped successfully' });
+    } catch (error) {
+        console.error('Error setting equipped instrument:', error);
+        res.status(500).json({ error: 'Internal server error' });
+    }
 };
📝 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.

export const setEquippedInstrumentController: RequestHandler = async (req, res) => {
    try {
        if (!req.body) {
            return res.status(400).json({ error: 'Request body is required' });
        }
        const accountId = await getAccountIdForRequest(req);
        if (!accountId) {
            return res.status(401).json({ error: 'Unauthorized' });
        }
        const inventory = await getInventory(accountId);
        const body = getJSONfromString(String(req.body)) as ISetEquippedInstrumentRequest;
        if (!body.Instrument || typeof body.Instrument !== 'string') {
            return res.status(400).json({ error: 'Invalid instrument value' });
        }
        inventory.EquippedInstrument = body.Instrument;
        await inventory.save();
        res.status(200).json({ message: 'Instrument equipped successfully' });
    } catch (error) {
        console.error('Error setting equipped instrument:', error);
        res.status(500).json({ error: 'Internal server error' });
    }
};
_:hammer_and_wrench: Refactor suggestion_ **Add error handling and input validation.** The controller needs to handle potential errors and validate input: 1. Add try-catch block for error handling 2. Validate the Instrument string 3. Set appropriate response status codes Apply this diff to improve error handling and validation: ```diff export const setEquippedInstrumentController: RequestHandler = async (req, res) => { + try { + if (!req.body) { + return res.status(400).json({ error: 'Request body is required' }); + } const accountId = await getAccountIdForRequest(req); + if (!accountId) { + return res.status(401).json({ error: 'Unauthorized' }); + } const inventory = await getInventory(accountId); const body = getJSONfromString(String(req.body)) as ISetEquippedInstrumentRequest; + if (!body.Instrument || typeof body.Instrument !== 'string') { + return res.status(400).json({ error: 'Invalid instrument value' }); + } inventory.EquippedInstrument = body.Instrument; await inventory.save(); - res.end(); + res.status(200).json({ message: 'Instrument equipped successfully' }); + } catch (error) { + console.error('Error setting equipped instrument:', error); + res.status(500).json({ error: 'Internal server error' }); + } }; ``` <!-- 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 export const setEquippedInstrumentController: RequestHandler = async (req, res) => { try { if (!req.body) { return res.status(400).json({ error: 'Request body is required' }); } const accountId = await getAccountIdForRequest(req); if (!accountId) { return res.status(401).json({ error: 'Unauthorized' }); } const inventory = await getInventory(accountId); const body = getJSONfromString(String(req.body)) as ISetEquippedInstrumentRequest; if (!body.Instrument || typeof body.Instrument !== 'string') { return res.status(400).json({ error: 'Invalid instrument value' }); } inventory.EquippedInstrument = body.Instrument; await inventory.save(); res.status(200).json({ message: 'Instrument equipped successfully' }); } catch (error) { console.error('Error setting equipped instrument:', error); res.status(500).json({ error: 'Internal server error' }); } }; ````` </details> <!-- suggestion_end --> <!-- This is an auto-generated comment by CodeRabbit -->
interface ISetEquippedInstrumentRequest {
Instrument: string;
}

2
src/routes/api.ts
View File

@ -56,6 +56,7 @@ import { sellController } from "@/src/controllers/api/sellController";
import { setActiveQuestController } from "@/src/controllers/api/setActiveQuestController";
import { setActiveShipController } from "@/src/controllers/api/setActiveShipController";
import { setBootLocationController } from "@/src/controllers/api/setBootLocationController";
import { setEquippedInstrumentController } from "@/src/controllers/api/setEquippedInstrumentController";
import { setPlacedDecoInfoController } from "@/src/controllers/api/setPlacedDecoInfoController";
import { setShipCustomizationsController } from "@/src/controllers/api/setShipCustomizationsController";
import { setShipFavouriteLoadoutController } from "@/src/controllers/api/setShipFavouriteLoadoutController";
@ -141,6 +142,7 @@ apiRouter.post("/purchase.php", purchaseController);
apiRouter.post("/rerollRandomMod.php", rerollRandomModController);
apiRouter.post("/saveLoadout.php", saveLoadoutController);
apiRouter.post("/sell.php", sellController);
apiRouter.post("/setEquippedInstrument.php", setEquippedInstrumentController);
apiRouter.post("/setPlacedDecoInfo.php", setPlacedDecoInfoController);
apiRouter.post("/setShipCustomizations.php", setShipCustomizationsController);
apiRouter.post("/setShipFavouriteLoadout.php", setShipFavouriteLoadoutController);

2
src/types/inventoryTypes/inventoryTypes.ts
View File

@ -250,7 +250,7 @@ export interface IInventoryResponse {
CompletedJobChains: ICompletedJobChain[];
SeasonChallengeHistory: ISeasonChallenge[];
MoaPets: IEquipmentDatabase[];
EquippedInstrument: string;
EquippedInstrument?: string;
InvasionChainProgress: IInvasionChainProgress[];
coderabbitai[bot] commented 2025-01-10 23:14:31 -08:00 (Migrated from github.com)
Review
Copy Link

💡 Codebase verification

Make EquippedInstrument a required field

The field is defined as required in the database schema (inventoryModel.ts) and the setEquippedInstrumentController implementation assumes its presence. The optional marker (?) should be removed to maintain consistency with the underlying data model.

  • src/types/inventoryTypes/inventoryTypes.ts: Change EquippedInstrument?: string; to EquippedInstrument: string;
🔗 Analysis chain

Verify if EquippedInstrument should be optional.

The property is marked as optional (?). Please verify if this is intentional:

  1. Are there cases where this property might not exist?
  2. Should existing inventories be migrated to include a default value?

Run the following script to check existing inventory documents:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if any existing inventories have the EquippedInstrument field
# and what values it contains

# Search for inventory-related files
echo "Searching for inventory-related files..."
rg -l "inventory" --type ts

# Search for migrations or default values
echo "Searching for any migrations or default values..."
rg "EquippedInstrument.*default" --type ts

Length of output: 3151

Script:

#!/bin/bash
# Check implementation and usage of EquippedInstrument

# Check setEquippedInstrumentController implementation
echo "Checking setEquippedInstrumentController implementation:"
ast-grep --pattern 'export class setEquippedInstrumentController {
  $$$
}'

# Search for EquippedInstrument usage
echo -e "\nSearching for EquippedInstrument usage:"
rg "EquippedInstrument" -B 2 -A 2 --type ts

# Check inventory models for related fields
echo -e "\nChecking inventory models:"
rg "EquippedInstrument" src/models/inventoryModels/ -B 2 -A 2

Length of output: 3911

_:bulb: Codebase verification_ **Make EquippedInstrument a required field** The field is defined as required in the database schema (inventoryModel.ts) and the setEquippedInstrumentController implementation assumes its presence. The optional marker (?) should be removed to maintain consistency with the underlying data model. - `src/types/inventoryTypes/inventoryTypes.ts`: Change `EquippedInstrument?: string;` to `EquippedInstrument: string;` <details> <summary>🔗 Analysis chain</summary> **Verify if EquippedInstrument should be optional.** The property is marked as optional (`?`). Please verify if this is intentional: 1. Are there cases where this property might not exist? 2. Should existing inventories be migrated to include a default value? Run the following script to check existing inventory documents: </details> <details> <summary>🏁 Scripts executed</summary> The following scripts were executed for the analysis: Script: ```shell #!/bin/bash # Description: Check if any existing inventories have the EquippedInstrument field # and what values it contains # Search for inventory-related files echo "Searching for inventory-related files..." rg -l "inventory" --type ts # Search for migrations or default values echo "Searching for any migrations or default values..." rg "EquippedInstrument.*default" --type ts ``` Length of output: 3151 --- Script: ```shell #!/bin/bash # Check implementation and usage of EquippedInstrument # Check setEquippedInstrumentController implementation echo "Checking setEquippedInstrumentController implementation:" ast-grep --pattern 'export class setEquippedInstrumentController { $$$ }' # Search for EquippedInstrument usage echo -e "\nSearching for EquippedInstrument usage:" rg "EquippedInstrument" -B 2 -A 2 --type ts # Check inventory models for related fields echo -e "\nChecking inventory models:" rg "EquippedInstrument" src/models/inventoryModels/ -B 2 -A 2 ``` Length of output: 3911 </details> <!-- This is an auto-generated comment by CodeRabbit -->
DataKnives: IEquipmentDatabase[];
Motorcycles: IEquipmentDatabase[];