feat: apply & track daily standing limit when trading in medallions #788

Merged
Sainan merged 4 commits from daily-standing-limit into main 2025-01-16 20:27:12 -08:00
Owner

Closes #786

Closes #786
coderabbitai[bot] commented 2025-01-16 09:06:09 -08:00 (Migrated from github.com)
Author
Owner

Walkthrough

The pull request introduces changes to the syndicate standing bonus controller and inventory service to enhance standing limit management. In the syndicateStandingBonusController, the code now uses a more structured approach with syndicateMeta to handle syndicate-related operations. The function getMaxStanding has been updated to accept an ISyndicate object instead of a string affiliation tag.

The inventory service receives new functions getStandingLimit and updateStandingLimit to manage standing limits. These functions use a mapping (standingLimitBinToInventoryKey) to interact with the inventory database, allowing for more precise control over standing limit tracking. The changes aim to improve the mechanism for tracking and updating standing limits when trading medallions.

Assessment against linked issues

Objective Addressed Explanation
Subtract from daily limit bin when trading medallions
Implement standing limit tracking mechanism

Finishing Touches

  • 📝 Generate Docstrings (Beta)

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.
<!-- This is an auto-generated comment: summarize by coderabbit.ai --> <!-- walkthrough_start --> ## Walkthrough The pull request introduces changes to the syndicate standing bonus controller and inventory service to enhance standing limit management. In the `syndicateStandingBonusController`, the code now uses a more structured approach with `syndicateMeta` to handle syndicate-related operations. The function `getMaxStanding` has been updated to accept an `ISyndicate` object instead of a string affiliation tag. The inventory service receives new functions `getStandingLimit` and `updateStandingLimit` to manage standing limits. These functions use a mapping (`standingLimitBinToInventoryKey`) to interact with the inventory database, allowing for more precise control over standing limit tracking. The changes aim to improve the mechanism for tracking and updating standing limits when trading medallions. ## Assessment against linked issues | Objective | Addressed | Explanation | |-----------|-----------|-------------| | Subtract from daily limit bin when trading medallions | ✅ | | | Implement standing limit tracking mechanism | ✅ | | <!-- walkthrough_end --> <!-- internal state start --> <!-- FAHghAtBAEAqAWBLAztF0CGA7TBXALgPYQDmAplmQE4b5kAm0AxoQLasX4Bc0ADldEIAjAFZkm+RADcyqIQE9mhetQxChifADoMiaFAB8wY+CjGAxOegAFAErQAyrnYYq84wjJ9cAGx/QqMgBHXFl8aEl8HwZoACIAMzJaHgxeXh9FADIImiYAa2h6XQzoZHxsekQsEmgfRFZNaAB3eAocjErqtBwOIr9EQixkWOaMVGRcIQb8OkYFCNboXGRqR10sbEwsRnREFSxJeMQY+fxFrGchVYB2AA5brThF/nrXRV5cKl5CFcF4hfQdjQqCIaFY6TIHAOmGgiVony8Z1omDSdVkW0Y+FyeVQGEKxUUZQqVRqdWmsMIAixHRJ3WgvQw/UGyEeCHQFHg2CYkM4wO6dG2MVB9X4hBkC0RNE6NWQ8jKkOg8woEyotKR4WW1Fx9FagQihBhyF44kQRxiZMagwliAERUQJWpQwwEgGQ26UkIPiktIZTKGWmMAEF6JVJINGRkADQA1AfPwBYKhMp8upYPJCg0oCZeC6sK4CO4ANmjVUqTFoas54UaHXogWQK1xpWNTFNiCYSiw3KoOEpCZCYQWyKaYz4gW9hGWJRoKBiVQlY8IYgkAHJUFnQu18tRWYsgeWcFdMExuQ2hfAqJOSPAF/FPT5CE1aQBVWwAGR4AG14DNeMguAA9ABJCaPAkxaCwrAAUazoUFUIhjNQMhUABDi8LBABy8EYA4SHUABcY+ABdy3AAugAFD++B/oBwGgeBkHQeh3JYNhKxUMhqHMWQWFYAhuEcfhhHEfcACUAbAIYJiQDAFCMIQ/wYAQxDkJQNCzEo7CcDw/CCKI4iSDIciKCwKg0Oomg6HoUmmDAbLrriOBKUQpAUKoGmQVC3ABBgTSlM4rBvIqJnKKoFnaLo+gQEY0lmMAyBUEwAEsAcl5+FqAGpIg0HyNs7a0GQDjlHl1QAEKDMsADCgxYveO74P+wCxM1wCWNAgYAJKuWpBWMBMLhuH8zCctUsgeIsTAjeQ66pcouDctAlB+fEuBdmGGx1PgigVPSyimooZAAB4oJIXQPiBHZPmc85nF4AAGsp5eWdBFcSZUVcg1WzelVB3buXgipSDXQJy4pXG0uC8EUGnCl2Pi4Co0B3eQ+CvSVJBvvUmh3RiSOQ9DhXFdKmPTDj8SXqwC53VUMgHJS8gCd63J3dGO13R1Di5WWBVkxTSMjlQ5MYBwEAfEIdRMBAR3fFQ+Ci/DyB/e1i1kH5UiuIgajREjj3c3QACyZDlDjuyzfQ80ZqURB6rdpRc/ldD0kbHS0Bg0YtO2N7ICKdRmriJ6yCCBq27rDtkGui7GrLxzINGgTpM6tKVIEEgJokgRduioJ3QAoodMuo/bz2yH941eOd7YUgI5Y+EwvgVl0yPrAwaPSjjnJyGQbSsHtZqYgaBD2ogABe92h8XhvG908odEN8dJLMJTOqe3uN3nBec09BWK48wahq6EbyNGts13XPgVlaClI4Fh040S6OLY+SyNjrRcFZPGAm0MdCz1ftsYPEI4dQL44HKDUZOBkMi7xVn5FK+8rSTXEDiNA/wm5VBbkTEkOMjrcgYCCRYFpwgqCOJQOYihkZG1biSEm2NWb0BEMsU6NQ0GkKodUHGy9KTSigU8Lw+MCr6gXPfaUtQsbVlQFgJ+I16DRDIcwCMtI7r8Jepg6oND8As2aEgSaCYE6nhhJ5TgDAICTnCLAAA8gAEXMQmEgrgRG2ztCUQhioqgSU8LCVaLorTexIBsfACIhoUPwPrDAh02EkHbqOcGTkfB0ECP3Y83JeDhE2OzTeesyA42EMuas38kjyUUlbVUXQ7xUkWAAoBmt1oRAwCQOOZB4jRBdF0PEyAkDxHCEQAW9BUDLFpGUKg80AkJPxOUKuiojbxOYOfVUW1caBSqOUKoagh5bQkm1QMcT3KuiDguFQTBZkgNQFfaWQMYh9jFhLaAnBNAx2MDASe8BlClEQH4+EeplEMB4HdFKyYUahPCaokg5FKlDxAbAOpPBBkkmPpoaIPBcz5lEoiy41Av460SslGqaVohUGQJlXg2Vx4FQieVC4n0cV1SoFoBqONABJhEjP54QAVhIieREldAeAczfnQOFUQyCorzNQFFi00W/TpA9LFKVao/QJVlHKW8VFvRIOSqqVKfq0sVpJaATyXm1m+UjFGET1EYqlUldiTNZAARppwemjN2xZJ1XqxgBr6A/K+SasRZqEoWqQo6gltq6ZuAdczHVHUDjUHiLBTAIZDXpN5VkyVAshYiyue2KW+cgby2WHdJqLVJJxV9dBf1p4bVYFptbBmpayBaq4Pm2IrUrCdW6u5GI/VAqDSvpNbA00y6FFNP8JZl5zb6JWMhRku1KhHGerswRttzUlo4gG8tlb7U1q1WTe0ZBowvE7faRQd466rxqFaf+IY7mX3+EtCI8hjSx07PfBqrNtieLWnOwI58YaZgrXawagUNjkC8tAm9W1jRIw6pYgkgZAHgvWorPkw65rcldQ+ao3tEa2yOidRRHUABi/TBiwECGMBEmidp4lA3e+6sAvXTFKlUE22oVCJNtoDWWqByZsH5q4VNZBRaTAlpmguOad7QGMBhVWj7irAx2itd9zJMB6mIegxJAG6leGEbSQhqArpIFAYsINVb/o62BeohjWBYCEAjWutwABpMg8gcbMuwOEQKf5oDq1VJOcYwLRHkg0G6UEt0bRKCoPWb4D90xyjpAuyD0HYPAPgzjMDta4BNANDe+T3ihjRmCXR7GuMlFQ1JWZ71rM9RIdHZbQIWJjjih2j3adigQ5+Z07Fwzv7g1HwTEaAy0gyA8I8flsrpM305YTCMt0eIb71GcKUABANI3kAEKaBcgW+SxAcLAQMGFLEdQwgAcQAPpvg6vrDqsBjulQO8djC5iMI51iNGQgt0qBPl+I0WrqoyBGQXOreGXguOUzY11qtz9KxeHc0SroKnSGSq02osRFmrM2b/fIBzTmTPFYJgVjR42akHJIy/VrKr/ONDKYaFsbYTjznmHiECtN/JCGpK0bYu7LzehY+t+c6BJHhC2ztvbB2TtnYu1dm7GE7sPaexJcxE6/DH1aL8Hto1UAci5IiRY2X1qMk0IoT9vVBGI9JGIh9EZHy0kpz3PUJAaAXFmZ2WVghkLunR5gGYqohAEHRB7bWiyDjrEhzc46ZQBlYmGYEv+nXbOKHU0B3kFcmAbObds9Sc7guLAOUc+DQ0zmywuQIdNHZbmSDGjq2A1HY0sZ+fFg9MGqnHJ9Viy1K6jPruXdyTdFeq9up+bR0bmgLPN79Z3617eQ0brpTqr6T7q/xpN+ZqoqOwf00xyPpdVrA2r8n2P7vMACMKacnG91RrKGD/wORCf8huVQfr4l6puzoyBZ4APlVS+sCiqReihHLea3b9j1DVrWnwPy8RqT7zxhK2VXRnUSvx3xv2gDrwyAbzgyf1cSwFfzxws2jAmBZxoDZ1P2/yoFFQ9D2A31bzLWvyAO72akbULQgGMGLVSwAPR0r3vVXVYOoxZAanrVoKbXai6lUjbT6gCiCm7SmnLw8WnSHTNgtnGDeVYhnVcynTbFnUU0z3ug6jR261sFkEixWC/niWjQWgoxDCbBvSWSjRjXZjv2QIfyb2xy0PgN0KNGZCTUsMFhjUkT8iOgFF6Qg1sPkBQKS12U0UsLtwbnAQJEwHsJqVFCjjL1QAgQkBKCWWDhj3dwizcOKUj0CBM3iOoESITC/UticW2liLQImB0VHDukCOCMf0GE0VqIS0b3WmsCkF4CaLqIqMGExiEBoDcHI1fVe11AfVBi8BiQTB7hkEYGBwg20KrRcP0KTQoz1G8L4FcHCGj00O6NaNCP5CsK73E2AD3kvQ2ijCpicNjyg3KCEEQkMMOK8A7iWEFCoBIEGC8FV2mg60RC4KGnHVUH8AKOjlkEeDQhNCUL8B6zumIyD2qBuKTWeMmK+JiDmPZn1kGHeIRJSwNGaLoCaLfEIA6HMQIGsHrCNgQzZlCXtCEEIFvhBgwDBi7hwC+VmL5hCwEAi363FGYMERhK4K0HMX0gkA6noCGMYDukO1wHtHoFFKiU7m7l7mODZO4zugAH4OpzEyC+S1S2DQShTck5T/oVcJDUBAgmkDJDQOk8kQYn5bpfhgSijXAvB45yTI16Bow6h0wShKcrgZhVhwj08rQ9NdVMTCBLFSoI4DSDJRTb1jQ5cFcfAldPjTSbksARoFpWshkJAo9/hQdY84z0R5g/lPQ9hIj8QD0Yi9irRoY8QdpzSqhg9eTu1Cj1hI5CiY4fi3dusxk1BEIDjPCu9jALBU94ljl50s9xAc851Tks0C95Ii9BNK5S97lw1I1ByvAICbCWjUDmQN9mCODus9SWCjyBSQDED1zjDNMFCPk+EoD41888kjCY0rj0cli3D9yuDDyq1jzvz6ZjzN1oBGV1jfC5JUBtz79qyhgOFX1njAhpj21ftASKyl4eicBHSY4AwYAI1nyFpfF/FAlWSflHyByrzECFj6Ybi+yDDf8koDzr9fyGKzyEMQyUS9lHSWsuD61oB9AkZYTGz4SCpuUMTqhwyeYgK+KZxWJBKuVoBsTgAeKYA7pCTiTSTySeDECVL6AST8AyTZAKSGUkYtKdK9KVgNKALoyRSxSFLeK7pqSfBaTDphLihHLDK7KXK6TX8BTLL8A5SbKlKpSZTRS1TuUtSxSJLJTpSfBZT6AQq4BvLhTfLrK+DC0YpbI0zCk8AXIhD1IYhDEDgeAaA/IO0gpizQpzINAIprJopQAZJNJphjs9hkBjtxxjgmgGBjsiRZYoo0q6qABOW4AAZgwAwD6oABZbgAAGAAJmmvuAAEZrhBrprCwyBBqABWJgdawsJgQsdajALuPq8aza6asgeasgaanq2qqAaAa4IQeIFa+axIMa+apga4EMea+a24QsegSa8sQa+ga4Sa+avqp664eIQsK4QsYGwsMaq666mATyTQRq3pFq37NqjquSHqoAA== --> <!-- internal state end --> <!-- finishing_touch_checkbox_start --> ## Finishing Touches - [ ] <!-- {"checkboxId": "7962f53c-55bc-4827-bfbf-6a18da830691"} --> 📝 Generate Docstrings (Beta) <!-- finishing_touch_checkbox_end --> <!-- tips_start --> --- 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> <!-- tips_end -->
coderabbitai[bot] (Migrated from github.com) reviewed 2025-01-16 09:07:57 -08:00
coderabbitai[bot] (Migrated from github.com) left a comment
Author
Owner

Actionable comments posted: 0

🧹 Nitpick comments (3)
src/services/inventoryService.ts (1)

524-532: Consider adding validation and improving type safety.

While the implementation is functional, consider these improvements:

  1. Add validation to prevent negative standing limits
  2. Use a type guard instead of type assertion for better type safety
 export const updateStandingLimit = (
     inventory: TInventoryDatabaseDocument,
     bin: TStandingLimitBin,
     subtrahend: number
 ): void => {
     if (bin != "STANDING_LIMIT_BIN_NONE") {
-        (inventory[standingLimitBinToInventoryKey[bin]] as number) -= subtrahend;
+        const key = standingLimitBinToInventoryKey[bin];
+        const currentLimit = inventory[key] as number;
+        inventory[key] = Math.max(0, currentLimit - subtrahend);
     }
 };
src/controllers/api/syndicateStandingBonusController.ts (2)

39-41: Consider adding error handling for standing calculations.

While the standing limit check is correct, consider adding validation for negative standing values and error handling for edge cases.

+    if (gainedStanding < 0) {
+        throw new Error(`Invalid negative standing gain: ${gainedStanding}`);
+    }
     if (gainedStanding > getStandingLimit(inventory, syndicateMeta.dailyLimitBin)) {
         gainedStanding = getStandingLimit(inventory, syndicateMeta.dailyLimitBin);
     }

     syndicate.Standing += gainedStanding;
     updateStandingLimit(inventory, syndicateMeta.dailyLimitBin, gainedStanding);

Also applies to: 45-45


Line range hint 71-79: Improve error handling and extract magic numbers.

Consider these improvements:

  1. Replace non-null assertions with proper error handling
  2. Extract the magic number 125000 as a named constant
+const LIBRARY_SYNDICATE_MAX_STANDING = 125000;
+
 const getMaxStanding = (syndicate: ISyndicate, title: number): number => {
     if (!syndicate.titles) {
         // LibrarySyndicate
-        return 125000;
+        return LIBRARY_SYNDICATE_MAX_STANDING;
     }
     if (title == 0) {
-        return syndicate.titles.find(x => x.level == 1)!.minStanding;
+        const firstTitle = syndicate.titles.find(x => x.level == 1);
+        if (!firstTitle) {
+            throw new Error(`No title found for level 1 in syndicate ${syndicate.name}`);
+        }
+        return firstTitle.minStanding;
     }
-    return syndicate.titles.find(x => x.level == title)!.maxStanding;
+    const currentTitle = syndicate.titles.find(x => x.level == title);
+    if (!currentTitle) {
+        throw new Error(`No title found for level ${title} in syndicate ${syndicate.name}`);
+    }
+    return currentTitle.maxStanding;
 };
📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 9633d307a2 and 983aa94802.

📒 Files selected for processing (2)
  • src/controllers/api/syndicateStandingBonusController.ts (3 hunks)
  • src/services/inventoryService.ts (3 hunks)
🔇 Additional comments (4)
src/services/inventoryService.ts (2)

497-515: LGTM! Well-structured mapping implementation.

The mapping between standing limit bins and inventory keys is comprehensive and type-safe.


517-522: LGTM! Clean implementation of standing limit retrieval.

The function handles all cases correctly, including the special case for "STANDING_LIMIT_BIN_NONE".

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

3-3: LGTM! Improved type safety and code organization.

Good use of ISyndicate type and centralized syndicate metadata access.

Also applies to: 6-6, 12-13


Line range hint 16-20: LGTM! Safe handling of medallion standing calculation.

Good use of optional chaining and clear standing calculation logic.

**Actionable comments posted: 0** <details> <summary>🧹 Nitpick comments (3)</summary><blockquote> <details> <summary>src/services/inventoryService.ts (1)</summary><blockquote> `524-532`: **Consider adding validation and improving type safety.** While the implementation is functional, consider these improvements: 1. Add validation to prevent negative standing limits 2. Use a type guard instead of type assertion for better type safety ```diff export const updateStandingLimit = ( inventory: TInventoryDatabaseDocument, bin: TStandingLimitBin, subtrahend: number ): void => { if (bin != "STANDING_LIMIT_BIN_NONE") { - (inventory[standingLimitBinToInventoryKey[bin]] as number) -= subtrahend; + const key = standingLimitBinToInventoryKey[bin]; + const currentLimit = inventory[key] as number; + inventory[key] = Math.max(0, currentLimit - subtrahend); } }; ``` </blockquote></details> <details> <summary>src/controllers/api/syndicateStandingBonusController.ts (2)</summary><blockquote> `39-41`: **Consider adding error handling for standing calculations.** While the standing limit check is correct, consider adding validation for negative standing values and error handling for edge cases. ```diff + if (gainedStanding < 0) { + throw new Error(`Invalid negative standing gain: ${gainedStanding}`); + } if (gainedStanding > getStandingLimit(inventory, syndicateMeta.dailyLimitBin)) { gainedStanding = getStandingLimit(inventory, syndicateMeta.dailyLimitBin); } syndicate.Standing += gainedStanding; updateStandingLimit(inventory, syndicateMeta.dailyLimitBin, gainedStanding); ``` Also applies to: 45-45 --- Line range hint `71-79`: **Improve error handling and extract magic numbers.** Consider these improvements: 1. Replace non-null assertions with proper error handling 2. Extract the magic number 125000 as a named constant ```diff +const LIBRARY_SYNDICATE_MAX_STANDING = 125000; + const getMaxStanding = (syndicate: ISyndicate, title: number): number => { if (!syndicate.titles) { // LibrarySyndicate - return 125000; + return LIBRARY_SYNDICATE_MAX_STANDING; } if (title == 0) { - return syndicate.titles.find(x => x.level == 1)!.minStanding; + const firstTitle = syndicate.titles.find(x => x.level == 1); + if (!firstTitle) { + throw new Error(`No title found for level 1 in syndicate ${syndicate.name}`); + } + return firstTitle.minStanding; } - return syndicate.titles.find(x => x.level == title)!.maxStanding; + const currentTitle = syndicate.titles.find(x => x.level == title); + if (!currentTitle) { + throw new Error(`No title found for level ${title} in syndicate ${syndicate.name}`); + } + return currentTitle.maxStanding; }; ``` </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 9633d307a20f972f272cb68c988bb56688bc153c and 983aa9480228817326e35c56c65aee9945c2e1e2. </details> <details> <summary>📒 Files selected for processing (2)</summary> * `src/controllers/api/syndicateStandingBonusController.ts` (3 hunks) * `src/services/inventoryService.ts` (3 hunks) </details> <details> <summary>🔇 Additional comments (4)</summary><blockquote> <details> <summary>src/services/inventoryService.ts (2)</summary> `497-515`: **LGTM! Well-structured mapping implementation.** The mapping between standing limit bins and inventory keys is comprehensive and type-safe. --- `517-522`: **LGTM! Clean implementation of standing limit retrieval.** The function handles all cases correctly, including the special case for "STANDING_LIMIT_BIN_NONE". </details> <details> <summary>src/controllers/api/syndicateStandingBonusController.ts (2)</summary> `3-3`: **LGTM! Improved type safety and code organization.** Good use of ISyndicate type and centralized syndicate metadata access. Also applies to: 6-6, 12-13 --- Line range hint `16-20`: **LGTM! Safe handling of medallion standing calculation.** Good use of optional chaining and clear standing calculation logic. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
coderabbitai[bot] (Migrated from github.com) reviewed 2025-01-16 20:22:49 -08:00
coderabbitai[bot] (Migrated from github.com) left a comment
Author
Owner

Actionable comments posted: 1

🧹 Nitpick comments (2)
src/types/inventoryTypes/inventoryTypes.ts (1)

114-129: Consider using a map for affiliations to enhance maintainability.

The IDailyAffiliations interface lists each affiliation as a separate property. To improve scalability and maintainability, consider using a map or an object with dynamic keys to represent affiliations. This approach simplifies adding or removing affiliations without modifying the interface structure.

src/services/inventoryService.ts (1)

518-518: Use strict equality checks for type safety and consistency.

In TypeScript, it's recommended to use strict equality === and strict inequality !== to ensure type-safe comparisons and maintain consistency across the codebase.

Apply this diff to update the equality checks:

 export const getStandingLimit = (inventory: IDailyAffiliations, bin: TStandingLimitBin): number => {
-    if (bin == "STANDING_LIMIT_BIN_NONE") {
+    if (bin === "STANDING_LIMIT_BIN_NONE") {
         return Number.MAX_SAFE_INTEGER;
     }
     return inventory[standingLimitBinToInventoryKey[bin]];
 };

 export const updateStandingLimit = (
     inventory: IDailyAffiliations,
     bin: TStandingLimitBin,
     subtrahend: number
 ): void => {
-    if (bin != "STANDING_LIMIT_BIN_NONE") {
+    if (bin !== "STANDING_LIMIT_BIN_NONE") {
         inventory[standingLimitBinToInventoryKey[bin]] -= subtrahend;
     }
 };

Also applies to: 529-529

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 983aa94802 and 7bf261fe41.

📒 Files selected for processing (2)
  • src/services/inventoryService.ts (3 hunks)
  • src/types/inventoryTypes/inventoryTypes.ts (1 hunks)
🔇 Additional comments (1)
src/types/inventoryTypes/inventoryTypes.ts (1)

Line range hint 16-20: Ensure all dependent code handles updated property types correctly.

The types for TrainingDate, LoadOutPresets, Mailbox, and GuildId have been changed to Date and Types.ObjectId. Please verify that all usage of these properties throughout the codebase accounts for these new types to prevent runtime errors or type mismatches.

Run the following script to identify code that may need updating:

Verification successful

Type changes are properly handled throughout the codebase

The verification confirms that all property type changes are correctly implemented and safely handled:

  • Database models use the new types (Date and Types.ObjectId)
  • Type conversions are consistently applied when transforming between database and response types
  • References are properly populated where needed
  • No type mismatches or unsafe operations were found
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Find usages of the changed properties to ensure they handle new types.

# Search for 'TrainingDate' usages
rg --type=typescript 'TrainingDate' -A 3 -B 3

# Search for 'LoadOutPresets' usages
rg --type=typescript 'LoadOutPresets' -A 3 -B 3

# Search for 'Mailbox' usages
rg --type=typescript 'Mailbox' -A 3 -B 3

# Search for 'GuildId' usages
rg --type=typescript 'GuildId' -A 3 -B 3

Length of output: 315


Script:

#!/bin/bash
# Search in TypeScript files for both type definitions and usage
# Using -t ts for TypeScript files and including more context lines

# Search for type definitions and usage
rg -t ts "TrainingDate|LoadOutPresets|Mailbox|GuildId" -B 5 -A 5

# Search specifically for property access patterns
rg -t ts "\.(TrainingDate|LoadOutPresets|Mailbox|GuildId)" -B 3 -A 3

# Search for type assertions or conversions related to these properties
rg -t ts "as.*?(TrainingDate|LoadOutPresets|Mailbox|GuildId)" -B 2 -A 2

Length of output: 29827

**Actionable comments posted: 1** <details> <summary>🧹 Nitpick comments (2)</summary><blockquote> <details> <summary>src/types/inventoryTypes/inventoryTypes.ts (1)</summary><blockquote> `114-129`: **Consider using a map for affiliations to enhance maintainability.** The `IDailyAffiliations` interface lists each affiliation as a separate property. To improve scalability and maintainability, consider using a map or an object with dynamic keys to represent affiliations. This approach simplifies adding or removing affiliations without modifying the interface structure. </blockquote></details> <details> <summary>src/services/inventoryService.ts (1)</summary><blockquote> `518-518`: **Use strict equality checks for type safety and consistency.** In TypeScript, it's recommended to use strict equality `===` and strict inequality `!==` to ensure type-safe comparisons and maintain consistency across the codebase. Apply this diff to update the equality checks: ```diff export const getStandingLimit = (inventory: IDailyAffiliations, bin: TStandingLimitBin): number => { - if (bin == "STANDING_LIMIT_BIN_NONE") { + if (bin === "STANDING_LIMIT_BIN_NONE") { return Number.MAX_SAFE_INTEGER; } return inventory[standingLimitBinToInventoryKey[bin]]; }; export const updateStandingLimit = ( inventory: IDailyAffiliations, bin: TStandingLimitBin, subtrahend: number ): void => { - if (bin != "STANDING_LIMIT_BIN_NONE") { + if (bin !== "STANDING_LIMIT_BIN_NONE") { inventory[standingLimitBinToInventoryKey[bin]] -= subtrahend; } }; ``` Also applies to: 529-529 </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 983aa9480228817326e35c56c65aee9945c2e1e2 and 7bf261fe41c7dd1186d0ca3d70191f7f6be60164. </details> <details> <summary>📒 Files selected for processing (2)</summary> * `src/services/inventoryService.ts` (3 hunks) * `src/types/inventoryTypes/inventoryTypes.ts` (1 hunks) </details> <details> <summary>🔇 Additional comments (1)</summary><blockquote> <details> <summary>src/types/inventoryTypes/inventoryTypes.ts (1)</summary> Line range hint `16-20`: **Ensure all dependent code handles updated property types correctly.** The types for `TrainingDate`, `LoadOutPresets`, `Mailbox`, and `GuildId` have been changed to `Date` and `Types.ObjectId`. Please verify that all usage of these properties throughout the codebase accounts for these new types to prevent runtime errors or type mismatches. Run the following script to identify code that may need updating: <details> <summary>✅ Verification successful</summary> <!-- [monologue] [task verify_type_changes_1/1] [observations] 1. Found type definitions and usage patterns for all properties: - TrainingDate: - Changed from IMongoDate to Date in IInventoryDatabase - Used in trainingResultController.ts with new Date() - Properly converted to IMongoDate in response using toMongoDate() - LoadOutPresets: - Changed from ILoadOutPresets to Types.ObjectId in IInventoryDatabase - Referenced in Schema as ObjectId with "Loadout" ref - Populated correctly in inventoryController.ts - Mailbox: - Changed from IMailbox to Types.ObjectId in IInventoryDatabase - Has proper schema definition with ObjectId type - IMailbox interface still exists for response type - GuildId: - Changed from ?IOid to ?Types.ObjectId in IInventoryDatabase - Used correctly in guild-related operations - Properly converted to IOid in response using toOid() [analysis] 1. All type changes are properly handled: - Database models use new types (Date, Types.ObjectId) - Controllers handle type conversions correctly - Response types maintain backward compatibility 2. Type conversion utilities are used consistently: - toMongoDate() for dates - toOid() for ObjectIds 3. Population is handled correctly for references 4. No type mismatches or unsafe operations found [actions] 1. Generate final response indicating verification success 2. Highlight the proper type handling throughout the codebase --> **Type changes are properly handled throughout the codebase** The verification confirms that all property type changes are correctly implemented and safely handled: - Database models use the new types (Date and Types.ObjectId) - Type conversions are consistently applied when transforming between database and response types - References are properly populated where needed - No type mismatches or unsafe operations were found </details> <details> <summary>🏁 Scripts executed</summary> The following scripts were executed for the analysis: Script: ```shell #!/bin/bash # Description: Find usages of the changed properties to ensure they handle new types. # Search for 'TrainingDate' usages rg --type=typescript 'TrainingDate' -A 3 -B 3 # Search for 'LoadOutPresets' usages rg --type=typescript 'LoadOutPresets' -A 3 -B 3 # Search for 'Mailbox' usages rg --type=typescript 'Mailbox' -A 3 -B 3 # Search for 'GuildId' usages rg --type=typescript 'GuildId' -A 3 -B 3 ``` Length of output: 315 --- Script: ```shell #!/bin/bash # Search in TypeScript files for both type definitions and usage # Using -t ts for TypeScript files and including more context lines # Search for type definitions and usage rg -t ts "TrainingDate|LoadOutPresets|Mailbox|GuildId" -B 5 -A 5 # Search specifically for property access patterns rg -t ts "\.(TrainingDate|LoadOutPresets|Mailbox|GuildId)" -B 3 -A 3 # Search for type assertions or conversions related to these properties rg -t ts "as.*?(TrainingDate|LoadOutPresets|Mailbox|GuildId)" -B 2 -A 2 ``` Length of output: 29827 </details> </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
coderabbitai[bot] (Migrated from github.com) commented 2025-01-16 20:22:48 -08:00
Author
Owner

🛠️ Refactor suggestion

Prevent standing limits from becoming negative.

Subtracting subtrahend from the current standing limit in updateStandingLimit may result in negative values. To maintain data integrity, add validation to ensure the standing limit does not fall below zero.

Apply this diff to prevent negative standing limits:

 export const updateStandingLimit = (
     inventory: IDailyAffiliations,
     bin: TStandingLimitBin,
     subtrahend: number
 ): void => {
     if (bin !== "STANDING_LIMIT_BIN_NONE") {
-        inventory[standingLimitBinToInventoryKey[bin]] -= subtrahend;
+        const key = standingLimitBinToInventoryKey[bin];
+        inventory[key] = Math.max(0, inventory[key] - subtrahend);
     }
 };
📝 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.

    if (bin !== "STANDING_LIMIT_BIN_NONE") {
        const key = standingLimitBinToInventoryKey[bin];
        inventory[key] = Math.max(0, inventory[key] - subtrahend);
    }
_:hammer_and_wrench: Refactor suggestion_ **Prevent standing limits from becoming negative.** Subtracting `subtrahend` from the current standing limit in `updateStandingLimit` may result in negative values. To maintain data integrity, add validation to ensure the standing limit does not fall below zero. Apply this diff to prevent negative standing limits: ```diff export const updateStandingLimit = ( inventory: IDailyAffiliations, bin: TStandingLimitBin, subtrahend: number ): void => { if (bin !== "STANDING_LIMIT_BIN_NONE") { - inventory[standingLimitBinToInventoryKey[bin]] -= subtrahend; + const key = standingLimitBinToInventoryKey[bin]; + inventory[key] = Math.max(0, inventory[key] - subtrahend); } }; ``` <!-- 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 if (bin !== "STANDING_LIMIT_BIN_NONE") { const key = standingLimitBinToInventoryKey[bin]; inventory[key] = Math.max(0, inventory[key] - subtrahend); } ````` </details> <!-- suggestion_end --> <!-- This is an auto-generated comment by CodeRabbit -->
Sign in to join this conversation.
No description provided.