feat(webui): delete account #615

Sainan · 2024-12-22T17:48:00-08:00

Sainan commented

2024-12-22 17:48:00 -08:00

Closes #612

coderabbitai[bot] commented

2024-12-22 17:48:06 -08:00

(Migrated from github.com)

Walkthrough

This pull request introduces a new account deletion feature that spans both server-side and client-side components of the application. The changes include the following:

A new controller function, deleteAccountController, has been added in src/controllers/custom/deleteAccountController.ts to handle account deletion requests. This function retrieves the account ID and performs concurrent deletions across multiple related models, including Account, Inventory, Loadout, PersonalRooms, and Ship.
A new route has been established in src/routes/custom.ts, allowing clients to send a GET request to the /deleteAccount endpoint, which is linked to the deleteAccountController.
In the web interface, a new menu item labeled "Delete Account" has been added to the user display name dropdown in static/webui/index.html, which triggers the account deletion process when clicked.
A corresponding JavaScript function, deleteAccount, has been introduced in static/webui/script.js. This function prompts the user for confirmation before sending a fetch request to the deletion endpoint and handles the logout and redirection upon successful deletion.

These changes collectively facilitate a user-friendly method for account deletion while ensuring that all associated data is removed efficiently.

📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bcb2c23775 and bb984255fc.

📒 Files selected for processing (1)

static/webui/index.html (2 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

static/webui/index.html

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

## Walkthrough This pull request introduces a new account deletion feature that spans both server-side and client-side components of the application. The changes include the following: - A new controller function, `deleteAccountController`, has been added in `src/controllers/custom/deleteAccountController.ts` to handle account deletion requests. This function retrieves the account ID and performs concurrent deletions across multiple related models, including Account, Inventory, Loadout, PersonalRooms, and Ship. - A new route has been established in `src/routes/custom.ts`, allowing clients to send a GET request to the `/deleteAccount` endpoint, which is linked to the `deleteAccountController`. - In the web interface, a new menu item labeled "Delete Account" has been added to the user display name dropdown in `static/webui/index.html`, which triggers the account deletion process when clicked. - A corresponding JavaScript function, `deleteAccount`, has been introduced in `static/webui/script.js`. This function prompts the user for confirmation before sending a fetch request to the deletion endpoint and handles the logout and redirection upon successful deletion. These changes collectively facilitate a user-friendly method for account deletion while ensuring that all associated data is removed efficiently.  --- <details> <summary>📜 Recent review details</summary> **Configuration used: .coderabbit.yaml** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between bcb2c237754178635b7e1ce34507ad75ab3ccc83 and bb984255fc1083bd302955f8e9d55f47ab2d6a00. </details> <details> <summary>📒 Files selected for processing (1)</summary> * `static/webui/index.html` (2 hunks) </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (1)</summary> * static/webui/index.html </details> </details>     --- Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? <details> <summary>❤️ Share</summary> - [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code) </details> <details> <summary>🪧 Tips</summary> ### Chat There are 3 ways to chat with [CodeRabbit](https://coderabbit.ai): - Review comments: Directly reply to a review comment made by CodeRabbit. Example: - `I pushed a fix in commit <commit_id>, please review it.` - `Generate unit testing code for this file.` - `Open a follow-up GitHub issue for this discussion.` - Files and specific lines of code (under the "Files changed" tab): Tag `@coderabbitai` in a new review comment at the desired location with your query. Examples: - `@coderabbitai generate unit testing code for this file.` - `@coderabbitai modularize this function.` - PR comments: Tag `@coderabbitai` in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples: - `@coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.` - `@coderabbitai read src/utils.ts and generate unit testing code.` - `@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.` - `@coderabbitai help me debug CodeRabbit configuration file.` Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. ### CodeRabbit Commands (Invoked using PR comments) - `@coderabbitai pause` to pause the reviews on a PR. - `@coderabbitai resume` to resume the paused reviews. - `@coderabbitai review` to trigger an incremental review. This is useful when automatic reviews are disabled for the repository. - `@coderabbitai full review` to do a full review from scratch and review all the files again. - `@coderabbitai summary` to regenerate the summary of the PR. - `@coderabbitai generate docstrings` to [generate docstrings](https://docs.coderabbit.ai/finishing-touches/docstrings) for this PR. ([Beta](https://docs.coderabbit.ai/early-access#beta)) - `@coderabbitai resolve` resolve all the CodeRabbit review comments. - `@coderabbitai configuration` to show the current CodeRabbit configuration for the repository. - `@coderabbitai help` to get help. ### Other keywords and placeholders - Add `@coderabbitai ignore` anywhere in the PR description to prevent this PR from being reviewed. - Add `@coderabbitai summary` to generate the high-level summary at a specific location in the PR description. - Add `@coderabbitai` anywhere in the PR title to generate the title automatically. ### Documentation and Community - Visit our [Documentation](https://docs.coderabbit.ai) for detailed information on how to use CodeRabbit. - Join our [Discord Community](http://discord.gg/coderabbit) to get help, request features, and share feedback. - Follow us on [X/Twitter](https://twitter.com/coderabbitai) for updates and announcements. </details>

coderabbitai[bot] (Migrated from github.com) reviewed 2024-12-22 17:50:14 -08:00

coderabbitai[bot] (Migrated from github.com) left a comment

Actionable comments posted: 5

🧹 Nitpick comments (1)

static/webui/script.js (1)
64-79: Consider improving user experience during account deletion.

The current implementation could benefit from better user feedback and confirmation.

Suggestions:

Add a more detailed confirmation message explaining the consequences

Show a loading spinner during deletion

Add a success message before redirect
 function deleteAccount() {
     if (
         window.confirm(
-            "Are you sure you want to delete your account " +
-                $(".displayname").text() +
-                " (" +
-                localStorage.getItem("email") +
-                ")? This action cannot be undone."
+            "WARNING: You are about to delete your account:\n\n" +
+            "Username: " + $(".displayname").text() + "\n" +
+            "Email: " + localStorage.getItem("email") + "\n\n" +
+            "This action will:\n" +
+            "- Permanently delete all your account data\n" +
+            "- Log you out immediately\n" +
+            "- Cannot be undone\n\n" +
+            "Are you sure you want to proceed?"
         )
     ) {
🧰 Tools

🪛 eslint

[error] 64-64: 'deleteAccount' is defined but never used.

(@typescript-eslint/no-unused-vars)

[error] 68-68: '$' is not defined.

(no-undef)

[error] 76-76: 'single' is not defined.

(no-undef)

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 066d07f8ba and bcb2c23775.

📒 Files selected for processing (4)

src/controllers/custom/deleteAccountController.ts (1 hunks)
src/routes/custom.ts (2 hunks)
static/webui/index.html (1 hunks)
static/webui/script.js (1 hunks)

🧰 Additional context used

🪛 eslint

static/webui/script.js

[error] 64-64: 'deleteAccount' is defined but never used.

(@typescript-eslint/no-unused-vars)

[error] 68-68: '$' is not defined.

(no-undef)

[error] 76-76: 'single' is not defined.

(no-undef)

🔇 Additional comments (1)

src/controllers/custom/deleteAccountController.ts (1)

11-17: Consider additional data cleanup

The current implementation might leave orphaned data in other collections.

Let's scan the codebase for other potential related collections:

Consider implementing a comprehensive cleanup strategy:

Identify all related collections
Add cleanup operations within the transaction
Document the cleanup process

**Actionable comments posted: 5** <details> <summary>🧹 Nitpick comments (1)</summary><blockquote> <details> <summary>static/webui/script.js (1)</summary><blockquote> `64-79`: **Consider improving user experience during account deletion.** The current implementation could benefit from better user feedback and confirmation. Suggestions: 1. Add a more detailed confirmation message explaining the consequences 2. Show a loading spinner during deletion 3. Add a success message before redirect ```diff function deleteAccount() { if ( window.confirm( - "Are you sure you want to delete your account " + - $(".displayname").text() + - " (" + - localStorage.getItem("email") + - ")? This action cannot be undone." + "WARNING: You are about to delete your account:\n\n" + + "Username: " + $(".displayname").text() + "\n" + + "Email: " + localStorage.getItem("email") + "\n\n" + + "This action will:\n" + + "- Permanently delete all your account data\n" + + "- Log you out immediately\n" + + "- Cannot be undone\n\n" + + "Are you sure you want to proceed?" ) ) { ``` <details> <summary>🧰 Tools</summary> <details> <summary>🪛 eslint</summary> [error] 64-64: 'deleteAccount' is defined but never used. (@typescript-eslint/no-unused-vars) --- [error] 68-68: '$' is not defined. (no-undef) --- [error] 76-76: 'single' is not defined. (no-undef) </details> </details> </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: .coderabbit.yaml** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 066d07f8bac5f60ed285f13ff15fddfd3e7ad14d and bcb2c237754178635b7e1ce34507ad75ab3ccc83. </details> <details> <summary>📒 Files selected for processing (4)</summary> * `src/controllers/custom/deleteAccountController.ts` (1 hunks) * `src/routes/custom.ts` (2 hunks) * `static/webui/index.html` (1 hunks) * `static/webui/script.js` (1 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 eslint</summary> <details> <summary>static/webui/script.js</summary> [error] 64-64: 'deleteAccount' is defined but never used. (@typescript-eslint/no-unused-vars) --- [error] 68-68: '$' is not defined. (no-undef) --- [error] 76-76: 'single' is not defined. (no-undef) </details> </details> </details> <details> <summary>🔇 Additional comments (1)</summary> <details> <summary>src/controllers/custom/deleteAccountController.ts (1)</summary> `11-17`: **Consider additional data cleanup** The current implementation might leave orphaned data in other collections. Let's scan the codebase for other potential related collections: Consider implementing a comprehensive cleanup strategy: 1. Identify all related collections 2. Add cleanup operations within the transaction 3. Document the cleanup process </details> </details> </details>

src/controllers/custom/deleteAccountController.ts

						
				@ -0,0 +16,4 @@

				        Ship.deleteOne({ ShipOwnerId: accountId })

				    ]);

				    res.end();

				};

coderabbitai[bot] (Migrated from github.com) commented

2024-12-22 17:50:13 -08:00

⚠️ Potential issue

Critical: Implement proper security measures and error handling

The current implementation has several security and reliability concerns:

Using GET for deletion operations violates HTTP semantics
No additional authentication verification
No confirmation token validation
Missing error handling
No transaction management

Consider implementing these improvements:

-export const deleteAccountController: RequestHandler = async (req, res) => {
+export const deleteAccountController: RequestHandler = async (req, res, next) => {
+    try {
+        // Verify confirmation token
+        const confirmationToken = req.body.confirmationToken;
+        if (!confirmationToken) {
+            return res.status(400).json({ error: 'Confirmation token required' });
+        }
+
         const accountId = await getAccountIdForRequest(req);
-        await Promise.all([
-            Account.deleteOne({ _id: accountId }),
-            Inventory.deleteOne({ accountOwnerId: accountId }),
-            Loadout.deleteOne({ loadoutOwnerId: accountId }),
-            PersonalRooms.deleteOne({ personalRoomsOwnerId: accountId }),
-            Ship.deleteOne({ ShipOwnerId: accountId })
-        ]);
-        res.end();
+        
+        // Start transaction
+        const session = await mongoose.startSession();
+        await session.withTransaction(async () => {
+            await Promise.all([
+                Account.deleteOne({ _id: accountId }, { session }),
+                Inventory.deleteOne({ accountOwnerId: accountId }, { session }),
+                Loadout.deleteOne({ loadoutOwnerId: accountId }, { session }),
+                PersonalRooms.deleteOne({ personalRoomsOwnerId: accountId }, { session }),
+                Ship.deleteOne({ ShipOwnerId: accountId }, { session })
+            ]);
+        });
+        
+        res.status(200).json({ message: 'Account deleted successfully' });
+    } catch (error) {
+        next(error);
+    }
 };

Committable suggestion skipped: line range outside the PR's diff.

_:warning: Potential issue_ **Critical: Implement proper security measures and error handling** The current implementation has several security and reliability concerns: 1. Using GET for deletion operations violates HTTP semantics 2. No additional authentication verification 3. No confirmation token validation 4. Missing error handling 5. No transaction management Consider implementing these improvements: ```diff -export const deleteAccountController: RequestHandler = async (req, res) => { +export const deleteAccountController: RequestHandler = async (req, res, next) => { + try { + // Verify confirmation token + const confirmationToken = req.body.confirmationToken; + if (!confirmationToken) { + return res.status(400).json({ error: 'Confirmation token required' }); + } + const accountId = await getAccountIdForRequest(req); - await Promise.all([ - Account.deleteOne({ _id: accountId }), - Inventory.deleteOne({ accountOwnerId: accountId }), - Loadout.deleteOne({ loadoutOwnerId: accountId }), - PersonalRooms.deleteOne({ personalRoomsOwnerId: accountId }), - Ship.deleteOne({ ShipOwnerId: accountId }) - ]); - res.end(); + + // Start transaction + const session = await mongoose.startSession(); + await session.withTransaction(async () => { + await Promise.all([ + Account.deleteOne({ _id: accountId }, { session }), + Inventory.deleteOne({ accountOwnerId: accountId }, { session }), + Loadout.deleteOne({ loadoutOwnerId: accountId }, { session }), + PersonalRooms.deleteOne({ personalRoomsOwnerId: accountId }, { session }), + Ship.deleteOne({ ShipOwnerId: accountId }, { session }) + ]); + }); + + res.status(200).json({ message: 'Account deleted successfully' }); + } catch (error) { + next(error); + } }; ``` > Committable suggestion skipped: line range outside the PR's diff.

src/routes/custom.ts

coderabbitai[bot] (Migrated from github.com) commented

2024-12-22 17:50:13 -08:00

⚠️ Potential issue

Use DELETE HTTP method and add security middleware

Using GET for account deletion violates REST principles and could lead to accidental deletions through browser prefetching.

Apply this change:

-customRouter.get("/deleteAccount", deleteAccountController);
+customRouter.delete("/deleteAccount",
+    validateConfirmationToken,
+    rateLimit({
+        windowMs: 60 * 60 * 1000, // 1 hour
+        max: 3 // limit each IP to 3 deletion requests per hour
+    }),
+    deleteAccountController
+);

Committable suggestion skipped: line range outside the PR's diff.

_:warning: Potential issue_ **Use DELETE HTTP method and add security middleware** Using GET for account deletion violates REST principles and could lead to accidental deletions through browser prefetching. Apply this change: ```diff -customRouter.get("/deleteAccount", deleteAccountController); +customRouter.delete("/deleteAccount", + validateConfirmationToken, + rateLimit({ + windowMs: 60 * 60 * 1000, // 1 hour + max: 3 // limit each IP to 3 deletion requests per hour + }), + deleteAccountController +); ``` > Committable suggestion skipped: line range outside the PR's diff.

static/webui/index.html

						
				@ -39,2 +39,4 @@

				                    <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>

coderabbitai[bot] (Migrated from github.com) commented

2024-12-22 17:50:13 -08:00

🛠️ Refactor suggestion

Enhance user safety and accessibility

The current implementation lacks:

Visual warning about the irreversible action
Confirmation dialog
Accessibility attributes
Loading state during deletion

Consider these improvements:

 <li><hr class="dropdown-divider"></li>
-<li><a class="dropdown-item" onclick="deleteAccount();">Delete Account</a></li>
+<li>
+    <button 
+        class="dropdown-item text-danger"
+        data-bs-toggle="modal"
+        data-bs-target="#deleteAccountModal"
+        aria-label="Delete account permanently"
+    >
+        Delete Account
+    </button>
+</li>

Add this modal for confirmation:

<div class="modal fade" id="deleteAccountModal" tabindex="-1" aria-labelledby="deleteAccountModalLabel" aria-hidden="true">
    <div class="modal-dialog">
        <div class="modal-content">
            <div class="modal-header">
                <h5 class="modal-title" id="deleteAccountModalLabel">Delete Account</h5>
                <button type="button" class="btn-close" data-bs-dismiss="modal" aria-label="Close"></button>
            </div>
            <div class="modal-body">
                <p class="text-danger">Warning: This action cannot be undone. All your data will be permanently deleted.</p>
                <p>Please type "DELETE" to confirm:</p>
                <input type="text" class="form-control" id="deleteConfirmation" />
            </div>
            <div class="modal-footer">
                <button type="button" class="btn btn-secondary" data-bs-dismiss="modal">Cancel</button>
                <button type="button" class="btn btn-danger" id="confirmDeleteAccount" disabled>Delete Account</button>
            </div>
        </div>
    </div>
</div>

_:hammer_and_wrench: Refactor suggestion_ **Enhance user safety and accessibility** The current implementation lacks: 1. Visual warning about the irreversible action 2. Confirmation dialog 3. Accessibility attributes 4. Loading state during deletion Consider these improvements: ```diff <li><hr class="dropdown-divider"></li> -<li><a class="dropdown-item" onclick="deleteAccount();">Delete Account</a></li> +<li> + <button + class="dropdown-item text-danger" + data-bs-toggle="modal" + data-bs-target="#deleteAccountModal" + aria-label="Delete account permanently" + > + Delete Account + </button> +</li> ``` Add this modal for confirmation: ```html <div class="modal fade" id="deleteAccountModal" tabindex="-1" aria-labelledby="deleteAccountModalLabel" aria-hidden="true"> <div class="modal-dialog"> <div class="modal-content"> <div class="modal-header"> <h5 class="modal-title" id="deleteAccountModalLabel">Delete Account</h5> <button type="button" class="btn-close" data-bs-dismiss="modal" aria-label="Close"></button> </div> <div class="modal-body"> <p class="text-danger">Warning: This action cannot be undone. All your data will be permanently deleted.</p> <p>Please type "DELETE" to confirm:</p> <input type="text" class="form-control" id="deleteConfirmation" /> </div> <div class="modal-footer"> <button type="button" class="btn btn-secondary" data-bs-dismiss="modal">Cancel</button> <button type="button" class="btn btn-danger" id="confirmDeleteAccount" disabled>Delete Account</button> </div> </div> </div> </div> ```

static/webui/script.js

						
				@ -64,0 +76,4 @@

				            single.loadRoute("/webui/"); // Show login screen

				        });

				    }

				}

coderabbitai[bot] (Migrated from github.com) commented

2024-12-22 17:50:13 -08:00

⚠️ Potential issue

Enhance error handling and security in account deletion.

The account deletion implementation needs improvements in several areas:

Error handling is missing for failed requests
Auth token in URL could be exposed in logs
No loading state during deletion

Consider applying these improvements:

 function deleteAccount() {
     if (
         window.confirm(
             "Are you sure you want to delete your account " +
                 $(".displayname").text() +
                 " (" +
                 localStorage.getItem("email") +
                 ")? This action cannot be undone."
         )
     ) {
-        fetch("/custom/deleteAccount?" + window.authz).then(function () {
-            logout();
-            single.loadRoute("/webui/"); // Show login screen
-        });
+        const button = document.querySelector("#delete-account-btn");
+        if (button) button.disabled = true;
+        
+        fetch("/custom/deleteAccount", {
+            method: 'POST',
+            headers: {
+                'Authorization': window.authz
+            }
+        })
+        .then(response => {
+            if (!response.ok) {
+                throw new Error('Account deletion failed');
+            }
+            logout();
+            single.loadRoute("/webui/");
+        })
+        .catch(error => {
+            alert("Failed to delete account: " + error.message);
+        })
+        .finally(() => {
+            if (button) button.disabled = false;
+        });
     }
 }

📝 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.

function deleteAccount() {
    if (
        window.confirm(
            "Are you sure you want to delete your account " +
                $(".displayname").text() +
                " (" +
                localStorage.getItem("email") +
                ")? This action cannot be undone."
        )
    ) {
        const button = document.querySelector("#delete-account-btn");
        if (button) button.disabled = true;
        
        fetch("/custom/deleteAccount", {
            method: 'POST',
            headers: {
                'Authorization': window.authz
            }
        })
        .then(response => {
            if (!response.ok) {
                throw new Error('Account deletion failed');
            }
            logout();
            single.loadRoute("/webui/");
        })
        .catch(error => {
            alert("Failed to delete account: " + error.message);
        })
        .finally(() => {
            if (button) button.disabled = false;
        });
    }
}

🧰 Tools

🪛 eslint

[error] 64-64: 'deleteAccount' is defined but never used.

(@typescript-eslint/no-unused-vars)

[error] 68-68: '$' is not defined.

(no-undef)

[error] 76-76: 'single' is not defined.

(no-undef)

💡 Codebase verification

Account deletion implementation needs error handling improvements

The verification revealed several issues that need attention:

Frontend: The fetch call lacks error handling and doesn't handle failed requests
```
fetch("/custom/deleteAccount?" + window.authz).then(function () {
```

Backend: The controller doesn't include try-catch blocks or error responses

export const deleteAccountController: RequestHandler = async (req, res) => {
  const accountId = await getAccountIdForRequest(req);
  await Promise.all([...]);
  res.end();
};

Both frontend and backend should handle cases where:
- Account ID retrieval fails
- Database operations fail
- Network requests fail
- Authentication errors occur

🔗 Analysis chain

Verify the account deletion implementation across the codebase.

Let's ensure the account deletion feature is properly integrated.

Let me generate additional verification scripts to check for error handling and response handling in both frontend and backend:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the account deletion implementation

# Test 1: Check if deleteAccount function is properly bound to a UI element
echo "Checking for deleteAccount button/link in HTML..."
rg -l "deleteAccount\(\)" --type html

# Test 2: Check for related backend route implementation
echo "Checking for deleteAccount route implementation..."
rg -l "deleteAccount" --type typescript --type javascript

# Test 3: Check for proper error handling in related files
echo "Checking error handling in related files..."
rg "deleteAccount" -A 5 -B 5

Length of output: 5308

Script:

#!/bin/bash
# Check frontend error handling for the fetch call
echo "Checking frontend error handling..."
rg "fetch\(.*deleteAccount.*\)" -A 10 -B 2

# Check backend error handling in the controller
echo "Checking backend error handling..."
ast-grep --pattern 'export const deleteAccountController: RequestHandler = async (req, res) => {
  $$$
}'

# Check if all necessary models are properly cleaned up
echo "Checking for all database models that might need cleanup..."
rg "mongoose\.model|Schema\(" --type typescript

Length of output: 2179

🧰 Tools

🪛 eslint

[error] 64-64: 'deleteAccount' is defined but never used.

(@typescript-eslint/no-unused-vars)

[error] 68-68: '$' is not defined.

(no-undef)

[error] 76-76: 'single' is not defined.

(no-undef)

_:warning: Potential issue_ **Enhance error handling and security in account deletion.** The account deletion implementation needs improvements in several areas: 1. Error handling is missing for failed requests 2. Auth token in URL could be exposed in logs 3. No loading state during deletion Consider applying these improvements: ```diff function deleteAccount() { if ( window.confirm( "Are you sure you want to delete your account " + $(".displayname").text() + " (" + localStorage.getItem("email") + ")? This action cannot be undone." ) ) { - fetch("/custom/deleteAccount?" + window.authz).then(function () { - logout(); - single.loadRoute("/webui/"); // Show login screen - }); + const button = document.querySelector("#delete-account-btn"); + if (button) button.disabled = true; + + fetch("/custom/deleteAccount", { + method: 'POST', + headers: { + 'Authorization': window.authz + } + }) + .then(response => { + if (!response.ok) { + throw new Error('Account deletion failed'); + } + logout(); + single.loadRoute("/webui/"); + }) + .catch(error => { + alert("Failed to delete account: " + error.message); + }) + .finally(() => { + if (button) button.disabled = false; + }); } } ```  <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 function deleteAccount() { if ( window.confirm( "Are you sure you want to delete your account " + $(".displayname").text() + " (" + localStorage.getItem("email") + ")? This action cannot be undone." ) ) { const button = document.querySelector("#delete-account-btn"); if (button) button.disabled = true; fetch("/custom/deleteAccount", { method: 'POST', headers: { 'Authorization': window.authz } }) .then(response => { if (!response.ok) { throw new Error('Account deletion failed'); } logout(); single.loadRoute("/webui/"); }) .catch(error => { alert("Failed to delete account: " + error.message); }) .finally(() => { if (button) button.disabled = false; }); } } ````` </details>  <details> <summary>🧰 Tools</summary> <details> <summary>🪛 eslint</summary> [error] 64-64: 'deleteAccount' is defined but never used. (@typescript-eslint/no-unused-vars) --- [error] 68-68: '$' is not defined. (no-undef) --- [error] 76-76: 'single' is not defined. (no-undef) </details> </details> --- _:bulb: Codebase verification_ **Account deletion implementation needs error handling improvements** The verification revealed several issues that need attention: * Frontend: The fetch call lacks error handling and doesn't handle failed requests ```javascript fetch("/custom/deleteAccount?" + window.authz).then(function () { ``` * Backend: The controller doesn't include try-catch blocks or error responses ```typescript export const deleteAccountController: RequestHandler = async (req, res) => { const accountId = await getAccountIdForRequest(req); await Promise.all([...]); res.end(); }; ``` * Both frontend and backend should handle cases where: - Account ID retrieval fails - Database operations fail - Network requests fail - Authentication errors occur <details> <summary>🔗 Analysis chain</summary> **Verify the account deletion implementation across the codebase.** Let's ensure the account deletion feature is properly integrated. --- Let me generate additional verification scripts to check for error handling and response handling in both frontend and backend: </details> <details> <summary>🏁 Scripts executed</summary> The following scripts were executed for the analysis: Script: ```shell #!/bin/bash # Description: Verify the account deletion implementation # Test 1: Check if deleteAccount function is properly bound to a UI element echo "Checking for deleteAccount button/link in HTML..." rg -l "deleteAccount" --type html # Test 2: Check for related backend route implementation echo "Checking for deleteAccount route implementation..." rg -l "deleteAccount" --type typescript --type javascript # Test 3: Check for proper error handling in related files echo "Checking error handling in related files..." rg "deleteAccount" -A 5 -B 5 ``` Length of output: 5308 --- Script: ```shell #!/bin/bash # Check frontend error handling for the fetch call echo "Checking frontend error handling..." rg "fetch$.*deleteAccount.*$" -A 10 -B 2 # Check backend error handling in the controller echo "Checking backend error handling..." ast-grep --pattern 'export const deleteAccountController: RequestHandler = async (req, res) => { $$$ }' # Check if all necessary models are properly cleaned up echo "Checking for all database models that might need cleanup..." rg "mongoose\.model|Schema\(" --type typescript ``` Length of output: 2179 </details> <details> <summary>🧰 Tools</summary> <details> <summary>🪛 eslint</summary> [error] 64-64: 'deleteAccount' is defined but never used. (@typescript-eslint/no-unused-vars) --- [error] 68-68: '$' is not defined. (no-undef) --- [error] 76-76: 'single' is not defined. (no-undef) </details> </details>

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: OpenWF/SpaceNinjaServer#615