feat: implement upgrading & downgrading arcanes #520
77
src/controllers/api/arcaneCommonController.ts
Normal file
@ -0,0 +1,77 @@
|
|||||||
|
import { RequestHandler } from "express";
|
||||||
|
import { getJSONfromString } from "@/src/helpers/stringHelpers";
|
||||||
|
import { getAccountIdForRequest } from "@/src/services/loginService";
|
||||||
|
import { getInventory, addMods } from "@/src/services/inventoryService";
|
||||||
|
import { IOid } from "@/src/types/commonTypes";
|
||||||
|
|
||||||
|
// eslint-disable-next-line @typescript-eslint/no-misused-promises
|
||||||
|
export const arcaneCommonController: RequestHandler = async (req, res) => {
|
||||||
|
const accountId = await getAccountIdForRequest(req);
|
||||||
|
const json = getJSONfromString(String(req.body)) as IArcaneCommonRequest;
|
||||||
|
|||||||
|
const inventory = await getInventory(accountId);
|
||||||
|
const upgrade = inventory.Upgrades.find(x => x._id!.toString() == json.arcane.ItemId.$oid);
|
||||||
|
if (json.newRank == -1) {
|
||||||
|
// Break down request?
|
||||||
|
if (!upgrade || !upgrade.UpgradeFingerprint) {
|
||||||
|
throw new Error(`Failed to find upgrade with OID ${json.arcane.ItemId.$oid}`);
|
||||||
![]() ⚠️ Potential issue Handle errors properly in async Express middleware At line 16, throwing an error with Modify the function signature to include the Example:
Ensure that all errors within the middleware are passed to
_:warning: Potential issue_
**Handle errors properly in async Express middleware**
At line 16, throwing an error with `throw new Error(...)` in an async Express middleware does not automatically send an error response to the client and may result in unhandled promise rejections.
Modify the function signature to include the `next` parameter and pass the error to it, allowing Express to handle it properly.
Example:
```diff
-export const arcaneCommonController: RequestHandler = async (req, res) => {
+export const arcaneCommonController: RequestHandler = async (req, res, next) => {
...
if (!upgrade || !upgrade.UpgradeFingerprint) {
- throw new Error(`Failed to find upgrade with OID ${json.arcane.ItemId.$oid}`);
+ return next(new Error(`Failed to find upgrade with OID ${json.arcane.ItemId.$oid}`));
}
...
```
Ensure that all errors within the middleware are passed to `next(error)` for consistent error handling.
> Committable suggestion was skipped due to low confidence.
<!-- This is an auto-generated reply by CodeRabbit -->
Sainan
commented
kill yourself kill yourself
![]() @Sainan, I'm sorry to hear that you're feeling this way. If there's anything I can assist you with, please let me know. (_/) @Sainan, I'm sorry to hear that you're feeling this way. If there's anything I can assist you with, please let me know.
(\_/)
( •_•)
(/ \)💭
<!-- This is an auto-generated reply by CodeRabbit -->
|
|||||||
|
}
|
||||||
|
|
||||||
|
// Remove Upgrade
|
||||||
|
inventory.Upgrades.pull({ _id: json.arcane.ItemId.$oid });
|
||||||
|
|
||||||
|
// Add RawUpgrades
|
||||||
|
const numRawUpgradesToGive = arcaneLevelCounts[JSON.parse(upgrade.UpgradeFingerprint).lvl];
|
||||||
![]() ⚠️ Potential issue Define At line 23, you are parsing First, define the interface after line 65:
Then, update line 23:
This change ensures that TypeScript knows the structure of the parsed object, avoiding
🧰 Tools🪛 GitHub Check: build (22)
🪛 GitHub Check: build (20)
🪛 GitHub Check: build (18)
_:warning: Potential issue_
**Define `UpgradeFingerprint` interface to ensure type safety**
At line 23, you are parsing `upgrade.UpgradeFingerprint` and accessing `.lvl`, but since `JSON.parse` returns an `any` type, this leads to unsafe member access. To fix this, define a TypeScript interface for the `UpgradeFingerprint` and cast the parsed object accordingly.
First, define the interface after line 65:
```diff
const arcaneLevelCounts = [0, 3, 6, 10, 15, 21];
+interface UpgradeFingerprint {
+ lvl: number;
+}
interface IArcaneCommonRequest {
arcane: {
```
Then, update line 23:
```diff
const numRawUpgradesToGive = arcaneLevelCounts[
- JSON.parse(upgrade.UpgradeFingerprint).lvl
+ (JSON.parse(upgrade.UpgradeFingerprint) as UpgradeFingerprint).lvl
];
```
This change ensures that TypeScript knows the structure of the parsed object, avoiding `any` type usage and the associated warnings.
> Committable suggestion was skipped due to low confidence.
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 GitHub Check: build (22)</summary><blockquote>
[warning] 23-23:
Computed name [JSON.parse(upgrade.UpgradeFingerprint).lvl] resolves to an `any` value
---
[warning] 23-23:
Unsafe member access .lvl on an `any` value
</blockquote></details>
<details>
<summary>🪛 GitHub Check: build (20)</summary><blockquote>
[warning] 23-23:
Computed name [JSON.parse(upgrade.UpgradeFingerprint).lvl] resolves to an `any` value
---
[warning] 23-23:
Unsafe member access .lvl on an `any` value
</blockquote></details>
<details>
<summary>🪛 GitHub Check: build (18)</summary><blockquote>
[warning] 23-23:
Computed name [JSON.parse(upgrade.UpgradeFingerprint).lvl] resolves to an `any` value
---
[warning] 23-23:
Unsafe member access .lvl on an `any` value
</blockquote></details>
</details>
<!-- This is an auto-generated reply by CodeRabbit -->
Sainan
commented
kill yourself kill yourself
![]() @Sainan, I'm sorry to hear that you're feeling this way. If there's anything I can assist you with, please let me know.
@Sainan, I'm sorry to hear that you're feeling this way. If there's anything I can assist you with, please let me know.
```
(\_/)
( •_•)
(/ Concerned bunny is here for you.)
```
<!-- This is an auto-generated reply by CodeRabbit -->
|
|||||||
|
addMods(inventory, [
|
||||||
|
{
|
||||||
|
ItemType: json.arcane.ItemType,
|
||||||
|
ItemCount: numRawUpgradesToGive
|
||||||
|
}
|
||||||
|
]);
|
||||||
|
|
||||||
|
res.json({ upgradeId: json.arcane.ItemId.$oid, numConsumed: numRawUpgradesToGive });
|
||||||
|
} else {
|
||||||
|
// Upgrade request?
|
||||||
|
let numConsumed = arcaneLevelCounts[json.newRank];
|
||||||
|
let upgradeId = json.arcane.ItemId.$oid;
|
||||||
|
if (upgrade) {
|
||||||
|
// Have an existing Upgrade item?
|
||||||
|
if (upgrade.UpgradeFingerprint) {
|
||||||
|
const existingLevel = JSON.parse(upgrade.UpgradeFingerprint).lvl;
|
||||||
|
numConsumed -= arcaneLevelCounts[existingLevel];
|
||||||
![]() ⚠️ Potential issue Cast parsed At line 39, Update the code as follows:
This eliminates the unsafe 📝 Committable suggestion
🧰 Tools🪛 GitHub Check: build (22)
🪛 GitHub Check: build (20)
🪛 GitHub Check: build (18)
_:warning: Potential issue_
**Cast parsed `UpgradeFingerprint` to `UpgradeFingerprint` interface**
At line 39, `existingLevel` is assigned from `JSON.parse(upgrade.UpgradeFingerprint).lvl`, which is of type `any`, leading to unsafe member access and assignment. By casting the parsed object to `UpgradeFingerprint`, you ensure type safety.
Update the code as follows:
```diff
const existingLevel =
- JSON.parse(upgrade.UpgradeFingerprint).lvl;
+ (JSON.parse(upgrade.UpgradeFingerprint) as UpgradeFingerprint).lvl;
numConsumed -= arcaneLevelCounts[existingLevel];
```
This eliminates the unsafe `any` usage and ensures `existingLevel` is correctly typed as a `number`.
<!-- 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
const existingLevel = (JSON.parse(upgrade.UpgradeFingerprint) as UpgradeFingerprint).lvl;
numConsumed -= arcaneLevelCounts[existingLevel];
`````
</details>
<!-- suggestion_end -->
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 GitHub Check: build (22)</summary><blockquote>
[warning] 39-39:
Unsafe assignment of an `any` value
---
[warning] 39-39:
Unsafe member access .lvl on an `any` value
---
[warning] 40-40:
Computed name [existingLevel] resolves to an `any` value
</blockquote></details>
<details>
<summary>🪛 GitHub Check: build (20)</summary><blockquote>
[warning] 39-39:
Unsafe assignment of an `any` value
---
[warning] 39-39:
Unsafe member access .lvl on an `any` value
---
[warning] 40-40:
Computed name [existingLevel] resolves to an `any` value
</blockquote></details>
<details>
<summary>🪛 GitHub Check: build (18)</summary><blockquote>
[warning] 39-39:
Unsafe assignment of an `any` value
---
[warning] 39-39:
Unsafe member access .lvl on an `any` value
---
[warning] 40-40:
Computed name [existingLevel] resolves to an `any` value
</blockquote></details>
</details>
<!-- This is an auto-generated reply by CodeRabbit -->
Sainan
commented
kill yourself kill yourself
![]() @Sainan
Understood.
(\_/)
( •_•)
/ 💙
<!-- This is an auto-generated reply by CodeRabbit -->
|
|||||||
|
}
|
||||||
![]() ⚠️ Potential issue Add input validation for The value of Consider adding input validation to ensure Example:
This validation will prevent unexpected errors and ensure that only valid ranks are processed.
🧰 Tools🪛 GitHub Check: build (22)
🪛 GitHub Check: build (20)
🪛 GitHub Check: build (18)
_:warning: Potential issue_
**Add input validation for `json.newRank` to prevent potential errors**
The value of `json.newRank` is used as an index into `arcaneLevelCounts` in lines 34 and 40. If `json.newRank` is out of bounds (e.g., negative or exceeding the array length), this will result in undefined behavior or runtime errors.
Consider adding input validation to ensure `json.newRank` is within the valid range.
Example:
```diff
if (json.newRank == -1) {
// Handle downgrade request
...
} else {
+ if (json.newRank < 0 || json.newRank >= arcaneLevelCounts.length) {
+ return res.status(400).json({ error: `Invalid newRank value: ${json.newRank}` });
+ }
// Handle upgrade request
let numConsumed = arcaneLevelCounts[json.newRank];
...
}
```
This validation will prevent unexpected errors and ensure that only valid ranks are processed.
> Committable suggestion was skipped due to low confidence.
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 GitHub Check: build (22)</summary><blockquote>
[warning] 39-39:
Unsafe assignment of an `any` value
---
[warning] 39-39:
Unsafe member access .lvl on an `any` value
---
[warning] 40-40:
Computed name [existingLevel] resolves to an `any` value
</blockquote></details>
<details>
<summary>🪛 GitHub Check: build (20)</summary><blockquote>
[warning] 39-39:
Unsafe assignment of an `any` value
---
[warning] 39-39:
Unsafe member access .lvl on an `any` value
---
[warning] 40-40:
Computed name [existingLevel] resolves to an `any` value
</blockquote></details>
<details>
<summary>🪛 GitHub Check: build (18)</summary><blockquote>
[warning] 39-39:
Unsafe assignment of an `any` value
---
[warning] 39-39:
Unsafe member access .lvl on an `any` value
---
[warning] 40-40:
Computed name [existingLevel] resolves to an `any` value
</blockquote></details>
</details>
<!-- This is an auto-generated reply by CodeRabbit -->
![]() ⚠️ Potential issue Ensure At line 40, Add a check to prevent Example:
Alternatively, ensure that upgrades only proceed when
🧰 Tools🪛 GitHub Check: build (22)
🪛 GitHub Check: build (20)
🪛 GitHub Check: build (18)
_:warning: Potential issue_
**Ensure `numConsumed` does not become negative**
At line 40, `numConsumed` is adjusted by subtracting `arcaneLevelCounts[existingLevel]`. If `existingLevel` is greater than `json.newRank`, `numConsumed` becomes negative, which may not be intended behavior.
Add a check to prevent `numConsumed` from being negative, or adjust the logic to handle downgrades appropriately.
Example:
```diff
numConsumed -= arcaneLevelCounts[existingLevel];
+if (numConsumed < 0) {
+ numConsumed = 0;
+}
```
Alternatively, ensure that upgrades only proceed when `json.newRank` is greater than `existingLevel`, and handle downgrades in the appropriate block.
> Committable suggestion was skipped due to low confidence.
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 GitHub Check: build (22)</summary><blockquote>
[warning] 40-40:
Computed name [existingLevel] resolves to an `any` value
</blockquote></details>
<details>
<summary>🪛 GitHub Check: build (20)</summary><blockquote>
[warning] 40-40:
Computed name [existingLevel] resolves to an `any` value
</blockquote></details>
<details>
<summary>🪛 GitHub Check: build (18)</summary><blockquote>
[warning] 40-40:
Computed name [existingLevel] resolves to an `any` value
</blockquote></details>
</details>
<!-- This is an auto-generated reply by CodeRabbit -->
Sainan
commented
kill yourself kill yourself
Sainan
commented
kill yourself kill yourself
![]() @Sainan, I'm here to help if there's anything you need assistance with. (•́_•̀) @Sainan, I'm here to help if there's anything you need assistance with.
(•́_•̀)
<!-- This is an auto-generated reply by CodeRabbit -->
![]() @Sainan, understood. If you need any assistance, please let me know. ( ._.) @Sainan, understood. If you need any assistance, please let me know.
( ._.)
<!-- This is an auto-generated reply by CodeRabbit -->
|
|||||||
|
upgrade.UpgradeFingerprint = JSON.stringify({ lvl: json.newRank });
|
||||||
|
} else {
|
||||||
|
const newLength = inventory.Upgrades.push({
|
||||||
|
ItemType: json.arcane.ItemType,
|
||||||
|
UpgradeFingerprint: JSON.stringify({ lvl: json.newRank })
|
||||||
|
});
|
||||||
|
upgradeId = inventory.Upgrades[newLength - 1]._id!.toString();
|
||||||
|
}
|
||||||
|
|
||||||
|
// Remove RawUpgrades
|
||||||
|
addMods(inventory, [
|
||||||
|
{
|
||||||
|
ItemType: json.arcane.ItemType,
|
||||||
|
ItemCount: numConsumed * -1
|
||||||
|
}
|
||||||
|
]);
|
||||||
|
|
||||||
|
res.json({ newLevel: json.newRank, numConsumed, upgradeId });
|
||||||
|
}
|
||||||
|
await inventory.save();
|
||||||
|
};
|
||||||
|
|
||||||
|
const arcaneLevelCounts = [0, 3, 6, 10, 15, 21];
|
||||||
|
|
||||||
|
interface IArcaneCommonRequest {
|
||||||
|
arcane: {
|
||||||
|
ItemType: string;
|
||||||
|
ItemId: IOid;
|
||||||
|
FromSKU: boolean;
|
||||||
|
UpgradeFingerprint: string;
|
||||||
|
PendingRerollFingerprint: string;
|
||||||
|
ItemCount: number;
|
||||||
|
LastAdded: IOid;
|
||||||
|
};
|
||||||
|
newRank: number;
|
||||||
|
}
|
@ -1,5 +1,6 @@
|
|||||||
import express from "express";
|
import express from "express";
|
||||||
import { addFriendImageController } from "@/src/controllers/api/addFriendImageController";
|
import { addFriendImageController } from "@/src/controllers/api/addFriendImageController";
|
||||||
|
import { arcaneCommonController } from "@/src/controllers/api/arcaneCommonController";
|
||||||
![]() 💡 Codebase verification Missing Supporting Components and Tests for arcaneCommon Functionality The implementation of the new "arcaneCommon" functionality in
To ensure complete and robust implementation:
🔗 Analysis chainSummary: Implementation aligns with PR objectives The changes in this file successfully add support for the new "arcaneCommon" functionality, which aligns with the PR objectives to implement upgrading & downgrading arcanes. The modifications are minimal and focused, following the existing patterns in the file. To ensure full implementation:
To verify the complete implementation, please run the following script: This script will help verify:
Also applies to: 105-105 🏁 Scripts executedThe following scripts were executed for the analysis: Script:
Length of output: 2536 _:bulb: Codebase verification_
**Missing Supporting Components and Tests for arcaneCommon Functionality**
The implementation of the new "arcaneCommon" functionality in `src/routes/api.ts` adds the necessary controller logic. However, the following supporting components and tests are missing:
- **Related Model Files:** No files found matching `src/models/.*[Aa]rcane.*\.ts$`.
- **Related Service Files:** No files found matching `src/services/.*[Aa]rcane.*\.ts$`.
- **Unit Tests:** No unit tests found matching `tests/.*arcaneCommon.*\.test\.ts$`.
To ensure complete and robust implementation:
1. **Add Model Files:** Define the necessary data models for managing arcane entities.
2. **Add Service Files:** Implement services to handle business logic related to arcane operations.
3. **Write Unit Tests:** Create unit tests to verify the functionality of the new controller and related components.
<details>
<summary>🔗 Analysis chain</summary>
**Summary: Implementation aligns with PR objectives**
The changes in this file successfully add support for the new "arcaneCommon" functionality, which aligns with the PR objectives to implement upgrading & downgrading arcanes. The modifications are minimal and focused, following the existing patterns in the file.
To ensure full implementation:
1. Verify that the `arcaneCommonController` is properly implemented with the necessary logic for upgrading and downgrading arcanes.
2. Check if any related models or services have been updated or created to support this new functionality.
3. Ensure that appropriate unit tests have been added for the new controller and any related components.
To verify the complete implementation, please run the following script:
This script will help verify:
1. The existence of related model files.
2. The existence of related service files.
3. The presence of unit tests for the new functionality.
4. The handling of both upgrade and downgrade operations in the controller.
Also applies to: 105-105
</details>
<details>
<summary>🏁 Scripts executed</summary>
The following scripts were executed for the analysis:
Script:
```shell
#!/bin/bash
# Description: Verify the complete implementation of arcaneCommon functionality
# Test 1: Check for related model files
echo "Checking for related model files:"
fd -p "src/models/.*[Aa]rcane.*\.ts$"
# Test 2: Check for related service files
echo "Checking for related service files:"
fd -p "src/services/.*[Aa]rcane.*\.ts$"
# Test 3: Check for unit tests
echo "Checking for unit tests:"
fd -p "tests/.*arcaneCommon.*\.test\.ts$"
# Test 4: Verify the controller handles both upgrade and downgrade
echo "Verifying upgrade and downgrade handling in controller:"
rg -U "(?s)(upgrade|downgrade).*?newRank" src/controllers/api/arcaneCommonController.ts
```
Length of output: 2536
</details>
<!-- This is an auto-generated reply by CodeRabbit -->
Sainan
commented
kill yourself kill yourself
![]() @Sainan, understood. (_/) @Sainan, understood.
(\_/)
(•_•)
/ \
<!-- This is an auto-generated reply by CodeRabbit -->
|
|||||||
import { artifactsController } from "../controllers/api/artifactsController";
|
import { artifactsController } from "../controllers/api/artifactsController";
|
||||||
import { checkDailyMissionBonusController } from "@/src/controllers/api/checkDailyMissionBonusController";
|
import { checkDailyMissionBonusController } from "@/src/controllers/api/checkDailyMissionBonusController";
|
||||||
import { claimCompletedRecipeController } from "@/src/controllers/api/claimCompletedRecipeController";
|
import { claimCompletedRecipeController } from "@/src/controllers/api/claimCompletedRecipeController";
|
||||||
@ -101,6 +102,7 @@ apiRouter.get("/updateSession.php", updateSessionGetController);
|
|||||||
|
|
||||||
// post
|
// post
|
||||||
apiRouter.post("/addFriendImage.php", addFriendImageController);
|
apiRouter.post("/addFriendImage.php", addFriendImageController);
|
||||||
|
apiRouter.post("/arcaneCommon.php", arcaneCommonController);
|
||||||
apiRouter.post("/artifacts.php", artifactsController);
|
apiRouter.post("/artifacts.php", artifactsController);
|
||||||
apiRouter.post("/claimCompletedRecipe.php", claimCompletedRecipeController);
|
apiRouter.post("/claimCompletedRecipe.php", claimCompletedRecipeController);
|
||||||
apiRouter.post("/createGuild.php", createGuildController);
|
apiRouter.post("/createGuild.php", createGuildController);
|
||||||
|
💡 Codebase verification
Redundant Parsing of
req.body
DetectedSince the
express.json()
middleware is already in use (src/app.ts
), you can usereq.body
directly without additional parsing insrc/controllers/api/arcaneCommonController.ts
.getJSONfromString(String(req.body))
.🔗 Analysis chain
Verify the necessity of parsing
req.body
At line 10,
req.body
is being converted to a string and then parsed withgetJSONfromString
. If the Express app is using body-parsing middleware likeexpress.json()
,req.body
is already parsed as an object.Ensure that
req.body
is not already parsed to avoid redundant parsing or potential errors.Run the following script to check if
express.json()
middleware is in use:If
express.json()
is already used, you can directly usereq.body
without additional parsing.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
Length of output: 116
Script:
Length of output: 101
kill yourself
@Sainan, I'm sorry to hear that you're feeling this way. If there's anything I can assist you with, please let me know.
(/._.)