Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New callouts to be on top #4781

Merged
merged 2 commits into from
Dec 12, 2024
Merged

Conversation

valentinyanakiev
Copy link
Member

@valentinyanakiev valentinyanakiev commented Dec 12, 2024

new callouts on top

Summary by CodeRabbit

  • New Features

    • Improved logic for determining the sort order when creating new callouts.
  • Bug Fixes

    • Enhanced error handling to ensure methods only operate on fully initialized collaborations.
  • Documentation

    • Minor adjustments made to comments for clarity and consistency.

Copy link
Contributor

coderabbitai bot commented Dec 12, 2024

Walkthrough

The changes in this pull request focus on the CollaborationService class within the collaboration.service.ts file. The createCalloutOnCollaboration method's logic for determining the sortOrder of new callouts has been modified to calculate the minimum sort order instead of adding to the maximum. Additionally, error handling in the getCalloutGroupNames method has been improved to throw an EntityNotInitializedException if necessary data is missing. Minor comment adjustments for clarity were also made, but these do not affect the code's functionality.

Changes

File Path Change Summary
src/domain/collaboration/collaboration/collaboration.service.ts - Modified createCalloutOnCollaboration to adjust sortOrder calculation.
- Updated error handling in getCalloutGroupNames to throw EntityNotInitializedException if data is missing.
- Minor comment adjustments for clarity.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant CollaborationService
    participant Callout

    User->>CollaborationService: createCalloutOnCollaboration(calloutData, userID)
    CollaborationService->>Callout: Calculate minimum sort order
    Callout-->>CollaborationService: Return new sort order
    CollaborationService-->>User: Return created callout
Loading

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. (Experiment)
  • @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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (1)
src/domain/collaboration/collaboration/collaboration.service.ts (1)

423-426: Consider adding boundaries for sort order values

While the implementation correctly places new callouts at the top, continuously subtracting 1 from the minimum sort order could lead to increasingly negative numbers. Consider implementing boundaries or normalization.

-        Math.min(
-          ...collaboration.callouts.map(callout => callout.sortOrder),
-          0 // Needed in case there are no callouts. In that case the first callout will have sortOrder = 1
-        ) - 1;
+        const MIN_SORT_ORDER = -1000;
+        const currentMin = Math.min(
+          ...collaboration.callouts.map(callout => callout.sortOrder),
+          0
+        );
+        Math.max(MIN_SORT_ORDER, currentMin - 1);
📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 15c1951 and 3b06c42.

📒 Files selected for processing (1)
  • src/domain/collaboration/collaboration/collaboration.service.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/domain/collaboration/collaboration/collaboration.service.ts (1)

Pattern src/**/*.{ts,js}: Review the TypeScript/JavaScript code for NestJS best practices, dependency injection, module structure, and potential bugs.

Context Files (Do Not Review):

  • docs/Design.md - Design overview of the project
  • docs/Pagination.md - Pagination design overview
  • docs/Developing.md - Development setup overview
  • docs/graphql-typeorm-usage.md - overview of GraphQL and TypeORM usage and how they are used together with NestJS in the project
  • docs/database-definitions.md - guidelines for creating TypeORM entity defnitions
  • src/core/error-handling/graphql.exception.filter.ts - GraphQL error handling
  • src/core/error-handling/http.exception.filter.ts - HTTP error handling
  • src/core/error-handling/rest.error.response.ts - REST error response
  • src/core/error-handling/unhandled.exception.filter.ts - Global exception handler

Guidelines:

  • Our project uses global exception handlers (UnhandledExceptionFilter), so avoid suggesting additional try/catch blocks unless handling specific cases.
  • Use NestJS latest documentation from https://docs.nestjs.com/ for reference on NestJS best practices.
  • Use TypeORM latest documentation from https://typeorm.io/ for reference on TypeORM best practices.
  • Refer to the design overview in the context files for better understanding.

@valentinyanakiev valentinyanakiev merged commit b4dd825 into develop Dec 12, 2024
3 checks passed
@valentinyanakiev valentinyanakiev deleted the reverse-new-callout-sort-order branch December 12, 2024 14:11
valentinyanakiev added a commit that referenced this pull request Dec 16, 2024
* wip

* moved dir to adapters

* poc wingback create customer

* poc wingback create customer

* getEntitlements

* comment added

* first pass at two new modules for TemplatesManager, TemplateDefault

* added templates manager to space; removed the SpaceDefaults entity (module still present until move all data to be via defaults

* added templatesManager to platform

* moved creating of default innovatin flow input to space defaults

* back out space type on Template; tidy up Template module to use switch statements

* created template applier module

* tidy up naming

* updated set of default template types

* fixed circular dependency; moved logic for creating collaboration input to space defaults

* removed loading of defaults from files for collaboration content

* removed code based addition of callouts, innovation flow states

* tidy up naming

* added loading of default templates at platform level in to bootstrap

* removed option to create new innovation flow template

* added in migration:

* loading in templates on bootstrap

* added field for collaboration templates on templatesSet; added lookup for templatesManager

* added mutation to create template from collaboration; added logic to prevent template used as default to be deleted; fixed removal of template set on template manager

* initial creation of license + entitlements modules

* add license into account

* updated account to have license service + use that in mutations checking limits, removing notion of soft limits

* ensure data is loaded properly on account for license checking

* added mutation to reset the license calculations on account, including new auth privilege to be able to do so

* renamed Licensing module to LicensingFramework module; trigger license reset on Account after assigning / removing license

* removed usage of LicenseEngine outside of license services on space or account

* renamed entitlement to licenseEntitlement as entity; first pass at migration

* fixed issues in migration

* fixed issues related to auth reset; tidied up loader creator imports

* fixed auth cascade for templates of type post

* license reset running

* reset licenses on space after adding / removing license plans

* removed need for license check in community; added entitlement check in roleset when adding a VC

* remove auth reset when assigning / removing license plans

* added License to RoleSet

* added license to collaboration

* tidied up retrieval of license for whiteboard; added license to collaboration in migration

* fix typo; fix space spec file

* fix additional tests

* moved tempaltesManager to last migration in the list

* fixed retrieval of template when creating collaboration

* added logging

* fixed bootstrap setting of templates

* refactored inputCreator to do the data loading closer to usage; fixed picking up of templates; fixed bootstrap usage of templates

* added ability to retrieve limits on entitlements + current usage

* updated field names on entitlements

* updated field names on entitlements

* fixed account mutaiton logic bug

* ensure that licenses are reset when assigning beta tester or vc campaign role to a user

* added reset all account licenses mutation

* fixed bug on space entitlements; refactored code to reduce duplication

* fixed url generation for templates inside of TempaltesManager

* fixed bootstrap order to create forum earlier

* ensure collaboration creation on template provides some defaults for callouts

* fix deletion of templates of type post

* ensure more data is defaulted inside of template service for collaboration; add setting of isTemplate field on Collaboration, and also on contained Callouts

* ensure isTempalte is passed to Collaboration entity

* fixed groups in bootstrap space template; updated signature for creating callout from collaboration

* fixed missing field

* fixed type on mutation to create from collaboration

* fixed typo

* fixed groups in bootstrap space template; updated signature for creating callout from collaboration

* fixed missing field

* fixed type on mutation to create from collaboration

* fixed typo

* reworked applying collaboraiton template to collaboration

* improved error message in wrong type of ID passed in

* fixed build

* made migration last in the list

* rename migration to be last

* removed read check when looking up collaboration

* track free / plus / premium space entitlements separately

* updated migration order

* removed duplicate migration

* moved auth reset to mutation for applying the template to another collaboration

* extend lookup of entitlement usage to cover new types

* updaed license policy to reflect new entitlements; made license engine work with entitlements, not license privileges; removed license privilege (no longer relevant)

* updated migration to not drop indexes already removed

* fix for license reset on space

* added license policy rule for free space credential

* ensure license entitlements are reset as part of the bootstrap

* fixed typo

* extended reset all to include resetting licenses on accounts + AI server; moved migration to be last

* Address pr comment

* Address PR feedback

* Address PR comment

* Address PR comments

* Address PR comments

* Address PR comment

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* Improved types & naming

* Address PR comments

* Fixed switch-case logic in entitlements

* Converge entitlements schema

* Remove unused AuthorizationPrivilege

* pass in spaceID on space authorization as reload the entity

* initial rework of the space authorization to clean up logic so that the Space does not know about the parent account authorization

* removed privileges on space that are no longer used there

* minor tidy up

* fixed logic check: space auth does need license entitlements

* renamed the two ways for assigning entitlements

* fix compilation:

* renaming; code setup

* take rabbit suggestion

* simplify usage of wingback api setup

* added flag to be able to disable wingback

* fix build

* fix build

* fix build

* TypeОRM default timezone to be UTC (#4732)

* typeorm default timezone to be UTC

* timezone read from the config

* Added comment about default timezone

---------

Co-authored-by: Neil Smyth <[email protected]>
Co-authored-by: Valentin Yanakiev <[email protected]>

* In-app notifications  (#4760)

* wip

* wip

* wip

* wip

* wip

* handled empty arrays

* fix for new member notification

* update state mutation

* rename

* wip

* wip

* wip

* wip

* connection to remote service

* wip

* new fields

* typeorm default timezone to be UTC

* timezone read from the config

* return sorted result by time triggered

* category is the proper type

* commentUrl on user mentioned

* graphql enums redefined

* notifications limited to beta testers only

* fixed tests

* renamed migrations to be on top

* version bump

* timezone changes reverted

* timezone changes reverted from config

* update state authorization

* removed notificationsAll query

* redundant code

* reviewer suggestions

* getAgent -> getAgentOrFail

* first set of review changes

* improved updating state

* introducing mixin to avoid CD

* removed unused extends from mixin

* added mixin for other concrete classes

---------

Co-authored-by: Svetoslav <[email protected]>
Co-authored-by: bobbykolev <[email protected]>

* Async invoke all engines ingest (#4766)

* Async invoke all engines ingest

* Reviewer suggestions

* Revert unused code

* notification dates properly serialized

* dataloader output order must match key order (#4770)

* Propagate anonymousReadAccess from parentAuthorization for Space

* dataloader can now resolve to null for nullable fields

* Fixed imports

* proper return type

* Update src/services/external/wingback/wingback.manager.ts

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* Auto stash before merge of "wingback" and "origin/wingback"

* Update src/platform/licensing/wingback-subscription/licensing.wingback.subscription.service.spec.ts

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* fixed comments / test issue

* IAN mention includes comment origin name (#4780)

* mention includes comment origin name

* package version bump

* added correct description to the field

* New callouts to be on top (#4781)

* introduce AlkemioVirtualContributorEngine for OpenAI generic and assistant engines

* update quickstart-services

* add powerpoint mimetypes (#4784)

* 7327 Don't allow nameIds on createSubspaceInput (#4785)

Co-authored-by: Valentin Yanakiev <[email protected]>

* correct contributor data loader

---------

Co-authored-by: Svetoslav <[email protected]>
Co-authored-by: Neil Smyth <[email protected]>
Co-authored-by: Neil Smyth <[email protected]>
Co-authored-by: Carlos Cano <[email protected]>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: bobbykolev <[email protected]>
Co-authored-by: Evgeni Dimitrov <[email protected]>
Co-authored-by: Vladimir Aleksiev <[email protected]>
Co-authored-by: Vladimir Aleksiev <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant