feat: daily reset for syndicate standing #582

Sainan · 2024-12-18T16:48:06-08:00

Sainan commented

2024-12-18 16:48:06 -08:00

No description provided.

coderabbitai[bot] commented

2024-12-18 16:48:12 -08:00

(Migrated from github.com)

Walkthrough

This pull request introduces several interconnected changes across multiple files to enhance account and inventory management functionality. The primary modifications include:

In the login service and types, a new LastLoginDay field is added to track the most recent login date for an account.
The inventoryController is updated to implement a daily reset mechanism. When a user logs in, the controller now checks if the current login day differs from the last login day, and if so, resets certain daily affiliation values to 16000.
The getAccountIdForRequest function is refactored to getAccountForRequest in the login service, now returning the entire account object instead of just the account ID.
Type definitions are updated across multiple files, including changes to IInventoryDatabase and IInventoryResponse interfaces, modifying how certain properties are typed (e.g., using Types.ObjectId instead of previous type definitions).
A new DailyAffiliationHex property is introduced in the inventory model and types, expanding the tracking capabilities for daily affiliations.

These changes appear to be part of a broader effort to improve account tracking, daily reset mechanics, and type safety in the application's data models.

Sequence Diagram

sequenceDiagram
    participant Client
    participant InventoryController
    participant LoginService
    participant AccountModel
    
    Client->>InventoryController: Request inventory
    InventoryController->>LoginService: Get account
    LoginService->>AccountModel: Retrieve account
    AccountModel-->>LoginService: Return account
    LoginService-->>InventoryController: Return account
    
    alt First login of the day
        InventoryController->>AccountModel: Update LastLoginDay
        InventoryController->>AccountModel: Reset daily affiliations
    end
    
    InventoryController->>Client: Return inventory JSON

#579: Modifications to inventoryController.ts related to inventoryJSON handling, which shares a similar scope of changes to the current pull request's modifications to the inventory controller.

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 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 several interconnected changes across multiple files to enhance account and inventory management functionality. The primary modifications include: 1. In the login service and types, a new `LastLoginDay` field is added to track the most recent login date for an account. 2. The `inventoryController` is updated to implement a daily reset mechanism. When a user logs in, the controller now checks if the current login day differs from the last login day, and if so, resets certain daily affiliation values to 16000. 3. The `getAccountIdForRequest` function is refactored to `getAccountForRequest` in the login service, now returning the entire account object instead of just the account ID. 4. Type definitions are updated across multiple files, including changes to `IInventoryDatabase` and `IInventoryResponse` interfaces, modifying how certain properties are typed (e.g., using `Types.ObjectId` instead of previous type definitions). 5. A new `DailyAffiliationHex` property is introduced in the inventory model and types, expanding the tracking capabilities for daily affiliations. These changes appear to be part of a broader effort to improve account tracking, daily reset mechanics, and type safety in the application's data models. ## Sequence Diagram ```mermaid sequenceDiagram participant Client participant InventoryController participant LoginService participant AccountModel Client->>InventoryController: Request inventory InventoryController->>LoginService: Get account LoginService->>AccountModel: Retrieve account AccountModel-->>LoginService: Return account LoginService-->>InventoryController: Return account alt First login of the day InventoryController->>AccountModel: Update LastLoginDay InventoryController->>AccountModel: Reset daily affiliations end InventoryController->>Client: Return inventory JSON ``` ## Possibly related PRs - **#579**: Modifications to `inventoryController.ts` related to `inventoryJSON` handling, which shares a similar scope of changes to the current pull request's modifications to the inventory controller.      --- 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 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-18 16:50:17 -08:00

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

Actionable comments posted: 2

🔭 Outside diff range comments (1)

src/services/loginService.ts (1)
Line range hint 47-65: Enhance getAccountForRequest implementation.

The function has two issues:

It only fetches the _id field but claims to return the full account

It's missing the LastLoginDay update logic needed for the daily reset feature

Apply these changes:
     const account = await Account.findOne(
         {
             _id: req.query.accountId,
             Nonce: req.query.nonce
-        },
-        "_id"
+        }
     );
     if (!account) {
         throw new Error("Invalid accountId-nonce pair");
     }
+    const today = Math.floor(Date.now() / (24 * 60 * 60 * 1000));
+    if (account.LastLoginDay !== today) {
+        account.LastLoginDay = today;
+        await account.save();
+    }
     return account;

🧹 Nitpick comments (2)

src/services/loginService.ts (1)

Line range hint 47-68: Consider implementing a dedicated service for daily resets.

The current changes add LastLoginDay tracking but don't implement the actual syndicate standing reset logic. Consider:

Creating a dedicated service (e.g., dailyResetService.ts) to handle all daily reset logic

Moving the LastLoginDay check and update logic there

Implementing the syndicate standing reset in that service

This would improve separation of concerns and make the reset logic more maintainable.

Would you like help designing this service?
src/controllers/api/inventoryController.ts (1)
32-53: Consider optimizing the daily reset implementation.

The daily reset logic could be improved in several ways:

Consider combining the two save operations into one

Consider extracting the magic number 16000 into a constant

Consider adding timezone handling for consistent reset timing

Here's a suggested optimization:
+ const DAILY_AFFILIATION_LIMIT = 16000;
+ 
  // Handle daily reset
  const today: number = Math.trunc(new Date().getTime() / 86400000);
  if (account.LastLoginDay != today) {
      account.LastLoginDay = today;
-     await account.save();
 
-     inventory.DailyAffiliation = 16000;
-     inventory.DailyAffiliationPvp = 16000;
-     inventory.DailyAffiliationLibrary = 16000;
-     inventory.DailyAffiliationCetus = 16000;
-     inventory.DailyAffiliationQuills = 16000;
-     inventory.DailyAffiliationSolaris = 16000;
-     inventory.DailyAffiliationVentkids = 16000;
-     inventory.DailyAffiliationVox = 16000;
-     inventory.DailyAffiliationEntrati = 16000;
-     inventory.DailyAffiliationNecraloid = 16000;
-     inventory.DailyAffiliationZariman = 16000;
-     inventory.DailyAffiliationKahl = 16000;
-     inventory.DailyAffiliationCavia = 16000;
-     inventory.DailyAffiliationHex = 16000;
+     Object.assign(inventory, {
+         DailyAffiliation: DAILY_AFFILIATION_LIMIT,
+         DailyAffiliationPvp: DAILY_AFFILIATION_LIMIT,
+         DailyAffiliationLibrary: DAILY_AFFILIATION_LIMIT,
+         DailyAffiliationCetus: DAILY_AFFILIATION_LIMIT,
+         DailyAffiliationQuills: DAILY_AFFILIATION_LIMIT,
+         DailyAffiliationSolaris: DAILY_AFFILIATION_LIMIT,
+         DailyAffiliationVentkids: DAILY_AFFILIATION_LIMIT,
+         DailyAffiliationVox: DAILY_AFFILIATION_LIMIT,
+         DailyAffiliationEntrati: DAILY_AFFILIATION_LIMIT,
+         DailyAffiliationNecraloid: DAILY_AFFILIATION_LIMIT,
+         DailyAffiliationZariman: DAILY_AFFILIATION_LIMIT,
+         DailyAffiliationKahl: DAILY_AFFILIATION_LIMIT,
+         DailyAffiliationCavia: DAILY_AFFILIATION_LIMIT,
+         DailyAffiliationHex: DAILY_AFFILIATION_LIMIT
+     });
-     await inventory.save();
+     await Promise.all([account.save(), inventory.save()]);
  }

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 259bfa1362 and 17f511359c.

📒 Files selected for processing (6)

src/controllers/api/inventoryController.ts (3 hunks)
src/models/inventoryModels/inventoryModel.ts (1 hunks)
src/models/loginModel.ts (1 hunks)
src/services/loginService.ts (2 hunks)
src/types/inventoryTypes/inventoryTypes.ts (1 hunks)
src/types/loginTypes.ts (1 hunks)

🔇 Additional comments (5)

src/services/loginService.ts (1)

67-68: LGTM! Clean implementation of getAccountIdForRequest.

The function properly reuses getAccountForRequest and extracts the ID.

src/controllers/api/inventoryController.ts (2)

15-17: LGTM: Account retrieval change is well-aligned with requirements.

The switch from getAccountIdForRequest to getAccountForRequest properly supports the daily reset functionality by providing access to the account's LastLoginDay field.

23-26: LGTM: Inventory query properly uses account ObjectId.

The query correctly uses account._id.toString() to find the inventory document.

src/types/inventoryTypes/inventoryTypes.ts (1)

278-278: LGTM: Type definition is consistent with existing pattern.

The DailyAffiliationHex property follows the same pattern as other daily affiliation properties in the interface.

src/models/inventoryModels/inventoryModel.ts (1)

638-638: LGTM: Schema definition matches existing pattern.

The DailyAffiliationHex field is correctly defined as a Number type, consistent with other daily affiliation fields in the schema.

**Actionable comments posted: 2** <details> <summary>🔭 Outside diff range comments (1)</summary><blockquote> <details> <summary>src/services/loginService.ts (1)</summary><blockquote> Line range hint `47-65`: **Enhance getAccountForRequest implementation.** The function has two issues: 1. It only fetches the `_id` field but claims to return the full account 2. It's missing the LastLoginDay update logic needed for the daily reset feature Apply these changes: ```diff const account = await Account.findOne( { _id: req.query.accountId, Nonce: req.query.nonce - }, - "_id" + } ); if (!account) { throw new Error("Invalid accountId-nonce pair"); } + const today = Math.floor(Date.now() / (24 * 60 * 60 * 1000)); + if (account.LastLoginDay !== today) { + account.LastLoginDay = today; + await account.save(); + } return account; ``` </blockquote></details> </blockquote></details> <details> <summary>🧹 Nitpick comments (2)</summary><blockquote> <details> <summary>src/services/loginService.ts (1)</summary><blockquote> Line range hint `47-68`: **Consider implementing a dedicated service for daily resets.** The current changes add LastLoginDay tracking but don't implement the actual syndicate standing reset logic. Consider: 1. Creating a dedicated service (e.g., `dailyResetService.ts`) to handle all daily reset logic 2. Moving the LastLoginDay check and update logic there 3. Implementing the syndicate standing reset in that service This would improve separation of concerns and make the reset logic more maintainable. Would you like help designing this service? </blockquote></details> <details> <summary>src/controllers/api/inventoryController.ts (1)</summary><blockquote> `32-53`: **Consider optimizing the daily reset implementation.** The daily reset logic could be improved in several ways: 1. Consider combining the two save operations into one 2. Consider extracting the magic number 16000 into a constant 3. Consider adding timezone handling for consistent reset timing Here's a suggested optimization: ```diff + const DAILY_AFFILIATION_LIMIT = 16000; + // Handle daily reset const today: number = Math.trunc(new Date().getTime() / 86400000); if (account.LastLoginDay != today) { account.LastLoginDay = today; - await account.save(); - inventory.DailyAffiliation = 16000; - inventory.DailyAffiliationPvp = 16000; - inventory.DailyAffiliationLibrary = 16000; - inventory.DailyAffiliationCetus = 16000; - inventory.DailyAffiliationQuills = 16000; - inventory.DailyAffiliationSolaris = 16000; - inventory.DailyAffiliationVentkids = 16000; - inventory.DailyAffiliationVox = 16000; - inventory.DailyAffiliationEntrati = 16000; - inventory.DailyAffiliationNecraloid = 16000; - inventory.DailyAffiliationZariman = 16000; - inventory.DailyAffiliationKahl = 16000; - inventory.DailyAffiliationCavia = 16000; - inventory.DailyAffiliationHex = 16000; + Object.assign(inventory, { + DailyAffiliation: DAILY_AFFILIATION_LIMIT, + DailyAffiliationPvp: DAILY_AFFILIATION_LIMIT, + DailyAffiliationLibrary: DAILY_AFFILIATION_LIMIT, + DailyAffiliationCetus: DAILY_AFFILIATION_LIMIT, + DailyAffiliationQuills: DAILY_AFFILIATION_LIMIT, + DailyAffiliationSolaris: DAILY_AFFILIATION_LIMIT, + DailyAffiliationVentkids: DAILY_AFFILIATION_LIMIT, + DailyAffiliationVox: DAILY_AFFILIATION_LIMIT, + DailyAffiliationEntrati: DAILY_AFFILIATION_LIMIT, + DailyAffiliationNecraloid: DAILY_AFFILIATION_LIMIT, + DailyAffiliationZariman: DAILY_AFFILIATION_LIMIT, + DailyAffiliationKahl: DAILY_AFFILIATION_LIMIT, + DailyAffiliationCavia: DAILY_AFFILIATION_LIMIT, + DailyAffiliationHex: DAILY_AFFILIATION_LIMIT + }); - await inventory.save(); + await Promise.all([account.save(), inventory.save()]); } ``` </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 259bfa136220e663fe5205dfb1db8b5a6630e71a and 17f511359cef62748677116684d658d4d50d7731. </details> <details> <summary>📒 Files selected for processing (6)</summary> * `src/controllers/api/inventoryController.ts` (3 hunks) * `src/models/inventoryModels/inventoryModel.ts` (1 hunks) * `src/models/loginModel.ts` (1 hunks) * `src/services/loginService.ts` (2 hunks) * `src/types/inventoryTypes/inventoryTypes.ts` (1 hunks) * `src/types/loginTypes.ts` (1 hunks) </details> <details> <summary>🔇 Additional comments (5)</summary> <details> <summary>src/services/loginService.ts (1)</summary> `67-68`: **LGTM! Clean implementation of getAccountIdForRequest.** The function properly reuses getAccountForRequest and extracts the ID. </details> <details> <summary>src/controllers/api/inventoryController.ts (2)</summary> `15-17`: **LGTM: Account retrieval change is well-aligned with requirements.** The switch from `getAccountIdForRequest` to `getAccountForRequest` properly supports the daily reset functionality by providing access to the account's LastLoginDay field. --- `23-26`: **LGTM: Inventory query properly uses account ObjectId.** The query correctly uses `account._id.toString()` to find the inventory document. </details> <details> <summary>src/types/inventoryTypes/inventoryTypes.ts (1)</summary> `278-278`: **LGTM: Type definition is consistent with existing pattern.** The `DailyAffiliationHex` property follows the same pattern as other daily affiliation properties in the interface. </details> <details> <summary>src/models/inventoryModels/inventoryModel.ts (1)</summary> `638-638`: **LGTM: Schema definition matches existing pattern.** The `DailyAffiliationHex` field is correctly defined as a Number type, consistent with other daily affiliation fields in the schema. </details> </details> </details>

src/models/loginModel.ts

						
				@ -36,2 +36,3 @@

				        Nonce: { type: Number, default: 0 }

				        Nonce: { type: Number, default: 0 },

				        LastLoginDay: { type: Number }

				    },

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

2024-12-18 16:50:16 -08:00

🛠️ Refactor suggestion

Consider adding a default value for LastLoginDay.

While the schema addition matches the interface, consider setting a default value for new accounts to ensure consistency.

Apply this change:

-        LastLoginDay: { type: Number }
+        LastLoginDay: { type: Number, default: () => Math.floor(Date.now() / (24 * 60 * 60 * 1000)) }

This sets the default to the current day number (days since epoch) when creating new accounts.

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

        Nonce: { type: Number, default: 0 },
        LastLoginDay: { type: Number, default: () => Math.floor(Date.now() / (24 * 60 * 60 * 1000)) }

_:hammer_and_wrench: Refactor suggestion_ **Consider adding a default value for LastLoginDay.** While the schema addition matches the interface, consider setting a default value for new accounts to ensure consistency. Apply this change: ```diff - LastLoginDay: { type: Number } + LastLoginDay: { type: Number, default: () => Math.floor(Date.now() / (24 * 60 * 60 * 1000)) } ``` This sets the default to the current day number (days since epoch) when creating new accounts.  <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 Nonce: { type: Number, default: 0 }, LastLoginDay: { type: Number, default: () => Math.floor(Date.now() / (24 * 60 * 60 * 1000)) } ````` </details>

src/types/loginTypes.ts

						
				@ -32,6 +32,7 @@ export interface IDatabaseAccount {

				    ConsentNeeded: boolean;

				    TrackedSettings: string[];

				    Nonce: number;

				    LastLoginDay?: number;

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

2024-12-18 16:50:16 -08:00

💡 Codebase verification

Potential backward compatibility issue found

The field is marked as optional in the interface (LastLoginDay?: number), but the MongoDB schema in loginModel.ts defines it as a required field without a default value (LastLoginDay: { type: Number }). This mismatch could cause runtime errors for existing accounts that don't have this field set.

src/models/loginModel.ts: Add a default value to the LastLoginDay schema field to maintain backward compatibility
src/controllers/api/inventoryController.ts: Add null check before comparing account.LastLoginDay with today

🔗 Analysis chain

LGTM! Verify backward compatibility.

The optional LastLoginDay field is properly typed and maintains backward compatibility with existing accounts.

Let's verify there are no required usages of this optional field:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for direct non-null assertions or required usages of LastLoginDay
rg -l "LastLoginDay!" || rg -l "LastLoginDay: number"

Length of output: 54

Script:

#!/bin/bash
# Search for any usage of LastLoginDay to verify it's treated as optional
rg "LastLoginDay" -A 2 -B 2

Length of output: 981

_:bulb: Codebase verification_ **Potential backward compatibility issue found** The field is marked as optional in the interface (`LastLoginDay?: number`), but the MongoDB schema in `loginModel.ts` defines it as a required field without a default value (`LastLoginDay: { type: Number }`). This mismatch could cause runtime errors for existing accounts that don't have this field set. - `src/models/loginModel.ts`: Add a default value to the `LastLoginDay` schema field to maintain backward compatibility - `src/controllers/api/inventoryController.ts`: Add null check before comparing `account.LastLoginDay` with `today` <details> <summary>🔗 Analysis chain</summary> **LGTM! Verify backward compatibility.** The optional `LastLoginDay` field is properly typed and maintains backward compatibility with existing accounts. Let's verify there are no required usages of this optional field: </details> <details> <summary>🏁 Scripts executed</summary> The following scripts were executed for the analysis: Script: ```shell #!/bin/bash # Search for direct non-null assertions or required usages of LastLoginDay rg -l "LastLoginDay!" || rg -l "LastLoginDay: number" ``` Length of output: 54 --- Script: ```shell #!/bin/bash # Search for any usage of LastLoginDay to verify it's treated as optional rg "LastLoginDay" -A 2 -B 2 ``` Length of output: 981 </details>

Sainan reviewed 2024-12-18 16:51:35 -08:00

src/types/loginTypes.ts

						
				@ -32,6 +32,7 @@ export interface IDatabaseAccount {

				    ConsentNeeded: boolean;

				    TrackedSettings: string[];

				    Nonce: number;

				    LastLoginDay?: number;

Sainan commented

2024-12-18 16:51:35 -08:00

Non-issue.

Sainan reviewed 2024-12-18 16:51:42 -08:00

src/models/loginModel.ts

						
				@ -36,2 +36,3 @@

				        Nonce: { type: Number, default: 0 }

				        Nonce: { type: Number, default: 0 },

				        LastLoginDay: { type: Number }

				    },

Sainan commented

2024-12-18 16:51:42 -08:00

Fuck off

coderabbitai[bot] (Migrated from github.com) reviewed 2024-12-18 16:51:51 -08:00

src/types/loginTypes.ts

						
				@ -32,6 +32,7 @@ export interface IDatabaseAccount {

				    ConsentNeeded: boolean;

				    TrackedSettings: string[];

				    Nonce: number;

				    LastLoginDay?: number;

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

2024-12-18 16:51:51 -08:00

@Sainan, understood. Acknowledged your decision; no further action will be taken on this matter.

@Sainan, understood. Acknowledged your decision; no further action will be taken on this matter.

coderabbitai[bot] (Migrated from github.com) reviewed 2024-12-18 16:52:05 -08:00

src/models/loginModel.ts

						
				@ -36,2 +36,3 @@

				        Nonce: { type: Number, default: 0 }

				        Nonce: { type: Number, default: 0 },

				        LastLoginDay: { type: Number }

				    },

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

2024-12-18 16:52:05 -08:00

@Sainan I’m sorry to see you upset. Let me know if there’s anything I can do to help.

(_ _)（ ; ; ）

@Sainan I’m sorry to see you upset. Let me know if there’s anything I can do to help. (\_ _)（ ; ; ）

coderabbitai[bot] (Migrated from github.com) reviewed 2024-12-18 17:11:13 -08:00

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

Actionable comments posted: 0

🧹 Nitpick comments (1)

src/controllers/api/loginController.ts (1)
75-75: Consider documenting the excluded fields

The response intentionally excludes sensitive fields (email, password) and LastLoginDay. Consider adding a comment explaining why LastLoginDay is excluded from the response, as its purpose isn't immediately obvious like the other excluded fields.
+    // Exclude sensitive fields and LastLoginDay (used internally for daily reset logic)
     const { email, password, LastLoginDay, ...databaseAccount } = account.toJSON();

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 17f511359c and 36fe9db781.

📒 Files selected for processing (1)

src/controllers/api/loginController.ts (2 hunks)

🔇 Additional comments (2)

src/controllers/api/loginController.ts (2)

39-39: Verify LastLoginDay management across the codebase

The addition of LastLoginDay to the excluded fields in the response appears to be part of the daily reset feature for syndicate standing. However, we should verify:

Where LastLoginDay is being updated
How it's being used for the daily reset logic

Let's check the related implementations:

Also applies to: 75-75

✅ Verification successful

Based on the results, I can now generate the final response since we have clear evidence of how LastLoginDay is being used in the codebase.

Excluding LastLoginDay from response is correct and consistent with its usage

The exclusion of LastLoginDay from the login response is correct as it's an internal field used for daily reset logic:

In inventoryController.ts, it's used to manage daily resets for affiliation (syndicate standing)
The field is properly updated during inventory operations when the day changes
It's consistently excluded from both login response paths in loginController.ts

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Find where LastLoginDay is being managed and used

# Check for LastLoginDay updates
echo "Checking for LastLoginDay updates:"
rg -A 3 "LastLoginDay.*=" 

# Check for LastLoginDay usage in daily reset logic
echo "Checking for daily reset logic using LastLoginDay:"
rg -A 5 "LastLoginDay.*daily|syndicate.*reset"

# Check schema definition
echo "Checking schema definition:"
ast-grep --pattern 'LastLoginDay: {
  $$$
}'

Length of output: 1457

39-39: Ensure consistent response structure for new accounts

The destructuring pattern { email, password, LastLoginDay, ...databaseAccount } assumes LastLoginDay exists in new accounts. Verify that createAccount properly initializes this field.

✅ Verification successful

LastLoginDay is optional and safely handled in the code

The destructuring pattern is safe because:

LastLoginDay is defined as an optional field in the account interface (LastLoginDay?: number)
The field is properly initialized in the model schema
The destructuring will handle both presence and absence of the field correctly

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check createAccount implementation

# Find createAccount function definition
ast-grep --pattern 'createAccount = async ($$$) => {
  $$$
}'

# Or if it's a regular function
ast-grep --pattern 'function createAccount($$$) {
  $$$
}'

Length of output: 124

Script:

#!/bin/bash
# Let's try a broader search for createAccount and account creation patterns
# Search for files containing 'createAccount'
rg -l "createAccount"

# Search for any function/method that creates accounts
ast-grep --pattern 'async $_ createAccount($$$) {
  $$$
}'

# Search for LastLoginDay initialization
rg "LastLoginDay" -A 3 -B 3

# Look for Account type/interface definition
ast-grep --pattern 'interface Account {
  $$$
}'

ast-grep --pattern 'type Account = {
  $$$
}'

Length of output: 2812

Script:

#!/bin/bash
# Let's check the account creation implementation in loginService
rg -A 10 "createAccount" src/services/loginService.ts

# Also check the account creation controller
rg -A 10 "createAccount" src/controllers/custom/createAccountController.ts

# Check the Account model initialization
rg "new Account\(" -A 5 -B 5

Length of output: 1990

Script:

#!/bin/bash
# Let's check the database account type definition and transformation
rg "IDatabaseAccount" -A 10 src/types/loginTypes.ts

# Check the account data transformation
rg "toDatabaseAccount" -A 10 -B 2

Length of output: 3224

**Actionable comments posted: 0** <details> <summary>🧹 Nitpick comments (1)</summary><blockquote> <details> <summary>src/controllers/api/loginController.ts (1)</summary><blockquote> `75-75`: **Consider documenting the excluded fields** The response intentionally excludes sensitive fields (`email`, `password`) and `LastLoginDay`. Consider adding a comment explaining why `LastLoginDay` is excluded from the response, as its purpose isn't immediately obvious like the other excluded fields. ```diff + // Exclude sensitive fields and LastLoginDay (used internally for daily reset logic) const { email, password, LastLoginDay, ...databaseAccount } = account.toJSON(); ``` </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 17f511359cef62748677116684d658d4d50d7731 and 36fe9db781a07ed5ea854bad4d263a48d19a66a9. </details> <details> <summary>📒 Files selected for processing (1)</summary> * `src/controllers/api/loginController.ts` (2 hunks) </details> <details> <summary>🔇 Additional comments (2)</summary> <details> <summary>src/controllers/api/loginController.ts (2)</summary> `39-39`: **Verify LastLoginDay management across the codebase** The addition of `LastLoginDay` to the excluded fields in the response appears to be part of the daily reset feature for syndicate standing. However, we should verify: 1. Where LastLoginDay is being updated 2. How it's being used for the daily reset logic Let's check the related implementations: Also applies to: 75-75 <details> <summary>✅ Verification successful</summary> Based on the results, I can now generate the final response since we have clear evidence of how `LastLoginDay` is being used in the codebase. **Excluding LastLoginDay from response is correct and consistent with its usage** The exclusion of `LastLoginDay` from the login response is correct as it's an internal field used for daily reset logic: - In `inventoryController.ts`, it's used to manage daily resets for affiliation (syndicate standing) - The field is properly updated during inventory operations when the day changes - It's consistently excluded from both login response paths in `loginController.ts` </details> <details> <summary>🏁 Scripts executed</summary> The following scripts were executed for the analysis: Script: ```shell #!/bin/bash # Description: Find where LastLoginDay is being managed and used # Check for LastLoginDay updates echo "Checking for LastLoginDay updates:" rg -A 3 "LastLoginDay.*=" # Check for LastLoginDay usage in daily reset logic echo "Checking for daily reset logic using LastLoginDay:" rg -A 5 "LastLoginDay.*daily|syndicate.*reset" # Check schema definition echo "Checking schema definition:" ast-grep --pattern 'LastLoginDay: { $$$ }' ``` Length of output: 1457 </details> --- `39-39`: **Ensure consistent response structure for new accounts** The destructuring pattern `{ email, password, LastLoginDay, ...databaseAccount }` assumes `LastLoginDay` exists in new accounts. Verify that `createAccount` properly initializes this field. <details> <summary>✅ Verification successful</summary> **LastLoginDay is optional and safely handled in the code** The destructuring pattern is safe because: 1. `LastLoginDay` is defined as an optional field in the account interface (`LastLoginDay?: number`) 2. The field is properly initialized in the model schema 3. The destructuring will handle both presence and absence of the field correctly </details> <details> <summary>🏁 Scripts executed</summary> The following scripts were executed for the analysis: Script: ```shell #!/bin/bash # Description: Check createAccount implementation # Find createAccount function definition ast-grep --pattern 'createAccount = async ($$$) => { $$$ }' # Or if it's a regular function ast-grep --pattern 'function createAccount($$$) { $$$ }' ``` Length of output: 124 --- Script: ```shell #!/bin/bash # Let's try a broader search for createAccount and account creation patterns # Search for files containing 'createAccount' rg -l "createAccount" # Search for any function/method that creates accounts ast-grep --pattern 'async $_ createAccount($$$) { $$$ }' # Search for LastLoginDay initialization rg "LastLoginDay" -A 3 -B 3 # Look for Account type/interface definition ast-grep --pattern 'interface Account { $$$ }' ast-grep --pattern 'type Account = { $$$ }' ``` Length of output: 2812 --- Script: ```shell #!/bin/bash # Let's check the account creation implementation in loginService rg -A 10 "createAccount" src/services/loginService.ts # Also check the account creation controller rg -A 10 "createAccount" src/controllers/custom/createAccountController.ts # Check the Account model initialization rg "new Account\(" -A 5 -B 5 ``` Length of output: 1990 --- Script: ```shell #!/bin/bash # Let's check the database account type definition and transformation rg "IDatabaseAccount" -A 10 src/types/loginTypes.ts # Check the account data transformation rg "toDatabaseAccount" -A 10 -B 2 ``` Length of output: 3224 </details> </details> </details> </details>

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: OpenWF/SpaceNinjaServer#582