OpenWF/SpaceNinjaServer
12
6
Fork 14
Code Issues 43 Pull Requests 4 Actions Packages Projects Releases Activity

feat(webui): rename account #616

Merged
Sainan merged 1 commits from rename-account into main 2024-12-23 05:37:08 -08:00
Conversation 2 Commits 1 Files Changed 4 +27 -2
4 changed files with 27 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit aa87ab87dd - Show all commits

13
src/controllers/custom/renameAccountController.ts Normal file
View File

@ -0,0 +1,13 @@
import { RequestHandler } from "express";
import { getAccountForRequest } from "@/src/services/loginService";
export const renameAccountController: RequestHandler = async (req, res) => {
coderabbitai[bot] commented 2024-12-22 18:46:59 -08:00 (Migrated from github.com)
Review
Copy Link

💡 Codebase verification

Add error handling for getAccountForRequest to maintain consistency

The getAccountForRequest function in loginService.ts already has robust error handling by throwing errors for invalid parameters. However, while inventoryController properly handles these errors with try-catch and returns a 400 status, renameAccountController lacks similar error handling. This inconsistency could lead to unhandled promise rejections and poor user experience.

  • Add try-catch block in renameAccountController.ts to handle potential errors from getAccountForRequest:
try {
    const account = await getAccountForRequest(req);
    // ... rest of the code
} catch (e) {
    res.status(400).send("Log-in expired");
    return;
}
🔗 Analysis chain

Implement robust error handling for account retrieval.
Currently, if getAccountForRequest fails or returns null, the code doesn't handle that scenario gracefully. Consider verifying that the account object is valid before proceeding.

Let me analyze the implementation of getAccountForRequest and its usage in renameAccountController. From the results, I can see that:

  1. getAccountForRequest already has built-in error handling:

    • It throws an error if accountId is missing
    • It throws an error if nonce is missing or invalid

  2. In renameAccountController, the function is used without try-catch, but in inventoryController, it's properly wrapped in try-catch with a 400 response.

Let me verify if there are any other usages or error handling patterns.

Let me try a different pattern to catch all error handling patterns around getAccountForRequest.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for getAccountForRequest calls to see if its return value is always validated.

rg -A 5 $'getAccountForRequest'

Length of output: 2514

Script:

#!/bin/bash
# Search for all try-catch blocks around getAccountForRequest
ast-grep --pattern 'try {
  $$$
  getAccountForRequest($$$)
  $$$
} catch ($$$) {
  $$$
}'

Length of output: 92

Script:

#!/bin/bash
# Search for all files containing getAccountForRequest and show more context
rg -B 5 -A 10 "getAccountForRequest\(.*\)"

Length of output: 2843

_:bulb: Codebase verification_ **Add error handling for getAccountForRequest to maintain consistency** The `getAccountForRequest` function in `loginService.ts` already has robust error handling by throwing errors for invalid parameters. However, while `inventoryController` properly handles these errors with try-catch and returns a 400 status, `renameAccountController` lacks similar error handling. This inconsistency could lead to unhandled promise rejections and poor user experience. - Add try-catch block in `renameAccountController.ts` to handle potential errors from `getAccountForRequest`: ```typescript try { const account = await getAccountForRequest(req); // ... rest of the code } catch (e) { res.status(400).send("Log-in expired"); return; } ``` <details> <summary>🔗 Analysis chain</summary> **Implement robust error handling for account retrieval.** Currently, if getAccountForRequest fails or returns null, the code doesn't handle that scenario gracefully. Consider verifying that the account object is valid before proceeding. --- Let me analyze the implementation of `getAccountForRequest` and its usage in `renameAccountController`. From the results, I can see that: 1. `getAccountForRequest` already has built-in error handling: - It throws an error if `accountId` is missing - It throws an error if `nonce` is missing or invalid 2. In `renameAccountController`, the function is used without try-catch, but in `inventoryController`, it's properly wrapped in try-catch with a 400 response. Let me verify if there are any other usages or error handling patterns. --- Let me try a different pattern to catch all error handling patterns around getAccountForRequest. </details> <details> <summary>🏁 Scripts executed</summary> The following scripts were executed for the analysis: Script: ```shell #!/bin/bash # Description: Check for getAccountForRequest calls to see if its return value is always validated. rg -A 5 $'getAccountForRequest' ``` Length of output: 2514 --- Script: ```shell #!/bin/bash # Search for all try-catch blocks around getAccountForRequest ast-grep --pattern 'try { $$$ getAccountForRequest($$$) $$$ } catch ($$$) { $$$ }' ``` Length of output: 92 --- Script: ```shell #!/bin/bash # Search for all files containing getAccountForRequest and show more context rg -B 5 -A 10 "getAccountForRequest\(.*\)" ``` Length of output: 2843 </details> <!-- This is an auto-generated comment by CodeRabbit -->
const account = await getAccountForRequest(req);
coderabbitai[bot] commented 2024-12-22 18:46:59 -08:00 (Migrated from github.com)
Review
Copy Link

⚠️ Potential issue

Add null-check for the account.
If the user is unauthenticated or the account can't be retrieved, account may be null. Add a guard to ensure you don't attempt to modify properties of a null object.

- const account = await getAccountForRequest(req);
+ const account = await getAccountForRequest(req);
+ if (!account) {
+   return res.status(401).json({ error: "Unauthorized" });
+ }
📝 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.

    const account = await getAccountForRequest(req);
    if (!account) {
      return res.status(401).json({ error: "Unauthorized" });
    }
_:warning: Potential issue_ **Add null-check for the account.** If the user is unauthenticated or the account can't be retrieved, account may be null. Add a guard to ensure you don't attempt to modify properties of a null object. ```diff - const account = await getAccountForRequest(req); + const account = await getAccountForRequest(req); + if (!account) { + return res.status(401).json({ error: "Unauthorized" }); + } ``` <!-- 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 account = await getAccountForRequest(req); if (!account) { return res.status(401).json({ error: "Unauthorized" }); } ````` </details> <!-- suggestion_end --> <!-- This is an auto-generated comment by CodeRabbit -->
if (typeof req.query.newname == "string") {
account.DisplayName = req.query.newname;
await account.save();
res.end();
} else {
res.status(400).end();
}
};

2
src/routes/custom.ts
View File

@ -5,6 +5,7 @@ import { getItemListsController } from "@/src/controllers/custom/getItemListsCon
import { pushArchonCrystalUpgradeController } from "@/src/controllers/custom/pushArchonCrystalUpgradeController";
import { popArchonCrystalUpgradeController } from "@/src/controllers/custom/popArchonCrystalUpgradeController";
import { deleteAccountController } from "@/src/controllers/custom/deleteAccountController";
import { renameAccountController } from "@/src/controllers/custom/renameAccountController";
import { createAccountController } from "@/src/controllers/custom/createAccountController";
import { addItemController } from "@/src/controllers/custom/addItemController";
@ -19,6 +20,7 @@ customRouter.get("/getItemLists", getItemListsController);
customRouter.get("/pushArchonCrystalUpgrade", pushArchonCrystalUpgradeController);
customRouter.get("/popArchonCrystalUpgrade", popArchonCrystalUpgradeController);
customRouter.get("/deleteAccount", deleteAccountController);
customRouter.get("/renameAccount", renameAccountController);
customRouter.post("/createAccount", createAccountController);
customRouter.post("/addItem", addItemController);

3
static/webui/index.html
View File

@ -39,7 +39,8 @@
<ul class="dropdown-menu dropdown-menu-end">
<li><a class="dropdown-item" href="/webui/" onclick="logout();">Logout</a></li>
<li><hr class="dropdown-divider"></li>
<li><a class="dropdown-item" onclick="deleteAccount();">Delete Account</a></li>
<li><a class="dropdown-item" href="#" onclick="event.preventDefault();renameAccount();">Rename Account</a></li>
<li><a class="dropdown-item" href="#" onclick="event.preventDefault();deleteAccount();">Delete Account</a></li>
</ul>
</li>
</ul>

11
static/webui/script.js
View File

@ -61,6 +61,15 @@ function logout() {
localStorage.removeItem("password");
}
function renameAccount() {
const newname = window.prompt("What would you like to change your account name to?");
if (newname) {
fetch("/custom/renameAccount?" + window.authz + "&newname=" + newname).then(() => {
$(".displayname").text(newname);
});
}
}
function deleteAccount() {
if (
window.confirm(
@ -71,7 +80,7 @@ function deleteAccount() {
")? This action cannot be undone."
)
) {
fetch("/custom/deleteAccount?" + window.authz).then(function () {
fetch("/custom/deleteAccount?" + window.authz).then(() => {
logout();
single.loadRoute("/webui/"); // Show login screen
});