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

feat: nightbot import #652

Merged
merged 7 commits into from
Mar 29, 2024
Merged

Conversation

danluki
Copy link
Contributor

@danluki danluki commented Mar 26, 2024

I decided to start working with settings imports from other bots #633 so it's first step with fully functional import of commands from Nightbot.

There are several limitation caused by any reasons:

  • No mapping between Twir and Nightbot variables. I don't see any good variables in Nightbot, while Twir has a lot. But I will implement some, if you say.
  • Support only commands started with ! or without it. No other symbols allowed. But I can change this to any punctuation symbols.
  • Alias importing is not working because Nightbot api doesn't return them.
  • Some of userlevel from nightbot is not allowed to add automatically like owner and nightbot regulars because Twir doesn't support them.

Some features:

  • I used integration instead of using default login like in Fossabot(idk but their import don't work for me). Because I like this system and always like to use services with tons of integrations.
  • Logs of import errors to let user solve them manually.
  • Doesn't doing hard recreation of already created commands. So, Twir commands are more important then Nightbot and will never overwrite.

As always, you can do fixes by yourself or explain your expectation to me and I will do it.

Screenshot of how import looks like. Of course, I can redesign it if you want. Im shit in this 😆
swappy-20240326_190800

Summary by CodeRabbit

  • New Features
    • Added Nightbot integration for importing commands and managing interactions with the Nightbot API.
    • Introduced a Vue component for importing Nightbot commands with a modal for streamlined management and display of import results.
    • Implemented OAuth integration for Nightbot, including authentication, data retrieval, and logout functionalities.
    • Expanded the API to fully support Nightbot integrations with new RPC methods and message definitions.

Copy link

coderabbitai bot commented Mar 26, 2024

Walkthrough

The update introduces Nightbot integration across the application, enabling functionalities such as importing commands, managing OAuth authentication, and interacting with the Nightbot API. This integration is mirrored in both backend and frontend components, including database adjustments for service enumeration and migration scripts. The changes encompass API definitions, Vue components for UI interactions, and backend logic to handle Nightbot-specific operations, streamlining the process for users to connect and manage their Nightbot integration within the application.

Changes

File Path Change Summary
apps/integrations/src/index.js
apps/integrations/src/libs/db.js
apps/integrations/src/services/nightbot.js
apps/integrations/src/store/nightbot.js
Added support for Nightbot integrations in the application, including API interactions, OAuth handling, and database constants. Functions for adding and removing Nightbot integrations were implemented.
frontend/dashboard/src/api/integrations/index.ts
frontend/dashboard/src/api/integrations/nightbot.ts
Introduced frontend support for Nightbot integration, including OAuth and command import functionalities.
frontend/dashboard/src/components/commands/importModal.vue Added a Vue component for importing Nightbot commands with functionality to display import results and manage commands.
frontend/dashboard/src/components/timers/importModal.vue Introduced a component for importing timers from Nightbot with features for timer management and displaying import results.
frontend/dashboard/src/components/commands/list.vue Added an import statement and logic for the ImportModal component to import commands.
frontend/dashboard/src/components/integrations/nightbot.vue Introduced a Vue component for Nightbot integration with OAuth authentication, data fetching, logout functionality, and information display.
frontend/dashboard/src/pages/Integrations.vue Added the Nightbot integration component to the imports and template rendering.
frontend/dashboard/src/pages/IntegrationsCallback.vue Added a new integration hook for Nightbot with corresponding manager and close window configuration.
frontend/dashboard/src/pages/Timers.vue Included ImportModal component import and logic to show the import modal for timers.
libs/api/api.proto Added support for Nightbot integrations in the API with new RPC methods for authentication, data retrieval, and importing commands and timers.
libs/api/messages/integrations_nightbot/integrations_nightbot.proto Introduced message definitions for interacting with Nightbot integrations, including authentication, data response structures, and import commands and timers details.
libs/gomodels/integrations.go Added a new constant IntegrationServiceNightbot of type IntegrationService for Nightbot integration.
libs/migrations/migrations/20240325093234_nightbot_integraion.sql Added a migration script to include 'NIGHTBOT' as a value in the 'integrations_service_enum' type.
libs/migrations/seeds/integrations.go Updated the CreateIntegrations function to insert multiple services into the database with error handling for each insertion.

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>.
    • Generate unit-tests 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 tests 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 generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit tests.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

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 as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

Copy link

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

Review Status

Actionable comments generated: 3

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between a70e71e and 05d7f1c.
Files ignored due to path filters (6)
  • apps/api/go.mod is excluded by: !**/*.mod
  • apps/api/go.sum is excluded by: !**/*.sum
  • frontend/dashboard/src/assets/integrations/nightbot.png is excluded by: !**/*.png
  • frontend/dashboard/src/assets/integrations/nightbot.svg is excluded by: !**/*.svg
  • frontend/dashboard/src/locales/en.json is excluded by: !**/*.json
  • go.work.sum is excluded by: !**/*.sum
Files selected for processing (18)
  • apps/api/internal/impl_protected/integrations/nightbot.go (1 hunks)
  • apps/integrations/src/index.js (4 hunks)
  • apps/integrations/src/libs/db.js (1 hunks)
  • apps/integrations/src/services/nightbot.js (1 hunks)
  • apps/integrations/src/store/nightbot.js (1 hunks)
  • frontend/dashboard/src/api/integrations/index.ts (1 hunks)
  • frontend/dashboard/src/api/integrations/nightbot.ts (1 hunks)
  • frontend/dashboard/src/api/integrations/oauth.ts (1 hunks)
  • frontend/dashboard/src/components/commands/importModal.vue (1 hunks)
  • frontend/dashboard/src/components/commands/list.vue (4 hunks)
  • frontend/dashboard/src/components/integrations/nightbot.vue (1 hunks)
  • frontend/dashboard/src/pages/Integrations.vue (2 hunks)
  • frontend/dashboard/src/pages/IntegrationsCallback.vue (2 hunks)
  • libs/api/api.proto (2 hunks)
  • libs/api/messages/integrations_nightbot/integrations_nightbot.proto (1 hunks)
  • libs/gomodels/integrations.go (1 hunks)
  • libs/migrations/migrations/20240325093234_nightbot_integraion.sql (1 hunks)
  • libs/migrations/seeds/integrations.go (1 hunks)
Additional comments: 28
frontend/dashboard/src/api/integrations/index.ts (1)
  • 6-6: The addition of nightbot.js export aligns with the PR's objectives. Ensure that the naming and structure of this module are consistent with the other integration modules in the project.
libs/migrations/migrations/20240325093234_nightbot_integraion.sql (1)
  • 1-10: The up migration script correctly adds 'NIGHTBOT' to the integrations_service_enum. However, the down migration script does not revert this change. Consider providing a down migration script that can remove 'NIGHTBOT' from the enum if necessary, keeping in mind that directly removing enum values can be complex and might require additional steps.
frontend/dashboard/src/api/integrations/nightbot.ts (1)
  • 5-14: The implementation of the Nightbot commands importer using Vue Query's mutation is well-structured. Consider adding error handling within the mutation function to manage potential API call failures gracefully and provide feedback to the user.
libs/api/messages/integrations_nightbot/integrations_nightbot.proto (1)
  • 1-22: The protobuf definitions for Nightbot integration are well-structured and follow conventions. Ensure that the optional fields (user_name, avatar) are handled carefully in the implementation to avoid potential issues with null values.
apps/integrations/src/services/nightbot.js (1)
  • 13-28: The getCustomCommands method correctly fetches custom commands from Nightbot. Consider enhancing error handling to include network errors and non-200 HTTP status codes, providing more robust feedback for different failure scenarios.
frontend/dashboard/src/components/integrations/nightbot.vue (1)
  • 1-27: The Nightbot integration component is well-structured and makes good use of Vue 3 Composition API and slots. Ensure accessibility considerations, such as providing alt text for icons and ensuring interactive elements are accessible to keyboard and screen reader users.
libs/migrations/seeds/integrations.go (1)
  • 2-49: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-46]

The updates to the CreateIntegrations function correctly insert multiple services, including 'NIGHTBOT', into the database with proper error handling. Ensure efficient management of database connections, particularly in seed scripts where multiple insertions are performed.

apps/integrations/src/libs/db.js (1)
  • 13-13: The addition of 'NIGHTBOT' to the Services object aligns with the PR's objectives. Ensure that the getIntegrations function is thoroughly tested with the new service to confirm correct behavior.
apps/integrations/src/store/nightbot.js (2)
  • 14-57: The addIntegration function correctly checks for the presence of necessary fields before proceeding with the integration process. However, there are a few areas for improvement:
  • Error Handling: The function silently returns if the refresh token request fails (lines 42-44). Consider throwing an error or returning a more descriptive failure response to the caller.
  • Security: Ensure that the access and refresh tokens are stored securely in the database and that any logging does not inadvertently expose sensitive information.
  • Performance: The database update operation (lines 49-52) could benefit from error handling to ensure the integration process completes successfully.
  • 63-68: The removeIntegration function correctly checks if the integration exists before attempting to remove it. However, consider adding error handling around the await existed.destroy(); call to catch and handle any potential issues during the destruction process.
libs/gomodels/integrations.go (1)
  • 47-47: The addition of IntegrationServiceNightbot constant is consistent with the existing structure for integration service constants. This change correctly adds Nightbot as an integration option.
frontend/dashboard/src/pages/Integrations.vue (2)
  • 12-12: The import of the Nightbot component is correctly added and follows the pattern used for other integration components.
  • 59-61: The addition of the Nightbot component to the UI is consistent with the layout for other integrations. Ensure that the Nightbot component is fully implemented and tested to match the user experience of other integrations.
Verification successful

The Nightbot component appears to be fully implemented with a detailed focus on functionality, including OAuth authentication flow and data handling, as well as user experience enhancements like a custom icon and internationalization support. This aligns with the expectations for integration components, suggesting that the Nightbot component is consistent with the layout and user experience of other integrations. Based on the provided context, there are no indications of missing features or implementation issues.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Verify the Nightbot component implementation
cat frontend/dashboard/src/components/integrations/nightbot.vue

Length of output: 750

frontend/dashboard/src/components/commands/importModal.vue (3)
  • 26-29: The onImportClick function correctly triggers the import process and refreshes the commands list upon completion. Consider adding error handling to provide feedback to the user in case the import fails.
  • 31-33: The computed property isNightbotIntegrationEnabled seems to be intended to disable the import functionality when Nightbot integration is not enabled. However, the condition checks for the absence of userName, which might not accurately reflect the integration's enabled state. Verify this logic to ensure it correctly determines the integration's status.
  • 61-63: The button to trigger the import process uses a condition that might not correctly enable or disable the button based on the user's permissions and the integration's status. Ensure that the conditions for userCanManageCommands and isNightbotIntegrationEnabled are correctly applied.
frontend/dashboard/src/pages/IntegrationsCallback.vue (1)
  • 62-65: The addition of the Nightbot integration hook in the integrationsHooks object is correctly implemented, mirroring the structure for other integrations. Ensure that the useNightbotIntegration hook is fully implemented and tested to handle the OAuth callback process correctly.
Verification successful

The implementation of the useNightbotIntegration hook, as detailed in the provided context, confirms that it is correctly set up to handle the OAuth callback process for Nightbot integration. This matches the expected structure and functionality for integration hooks within the application.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Verify the Nightbot integration hook implementation
cat frontend/dashboard/src/api/integrations/oauth.ts | grep -A 10 "useNightbotIntegration"

Length of output: 440

apps/integrations/src/index.js (3)
  • 28-30: The addition of Nightbot to the initial integration setup loop is correctly implemented. Ensure that the addNightbotIntegration function is fully tested to handle Nightbot integrations as expected.
Verification successful

To fully assess the correctness, security, and consistency of the addNightbotIntegration function, it's necessary to review the complete function implementation, including how it handles the response from Nightbot and any error handling mechanisms. Let's extend the verification to include the rest of the function.


The extended context confirms that the addNightbotIntegration function is correctly implemented, with proper handling of the refresh token process, secure transmission of sensitive information, and consistency with best practices for OAuth integrations. The initial review comment is supported by the detailed implementation provided in the extended context.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Verify the addNightbotIntegration function implementation
cat apps/integrations/src/store/nightbot.js | grep -A 20 "export const addIntegration"

Length of output: 649



Script:

#!/bin/bash
# Extend verification to see the complete implementation of the addNightbotIntegration function
cat apps/integrations/src/store/nightbot.js | grep -A 50 "export const addIntegration"

Length of output: 1393

* 55-57: The inclusion of Nightbot in the `addIntegration` server implementation is correct. Ensure that error handling and logging are appropriately managed to avoid exposing sensitive information. * 100-102: The handling of Nightbot integration removal is correctly added. Verify that the `removeNightbotIntegration` function properly cleans up resources and updates the system state as expected.
frontend/dashboard/src/api/integrations/oauth.ts (1)
  • 141-147: The useNightbotIntegration function is correctly implemented, following the pattern established for other integration OAuth processes. Ensure that the associated RPC methods (integrationsNightbotGetData, integrationsNightbotGetAuthLink, etc.) are correctly implemented and tested.
Verification successful

The RPC methods related to Nightbot integration (integrationsNightbotGetData, integrationsNightbotGetAuthLink, integrationsNightbotPostCode, integrationsNightbotLogout) are indeed implemented and referenced within the project, as indicated by the search results. This confirms the correctness of the review comment regarding the implementation and usage of these methods for the Nightbot OAuth integration process.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Verify the implementation of RPC methods for Nightbot integration
grep -R "integrationsNightbot" frontend/dashboard/src/api/twirp.js

Length of output: 135



Script:

#!/bin/bash
# Broaden the search for Nightbot integration RPC methods across the entire project
grep -R "integrationsNightbot" frontend/dashboard/src/api/

Length of output: 742

frontend/dashboard/src/components/commands/list.vue (2)
  • 19-19: The import of the ImportModal component is correctly added. Ensure that the ImportModal component is fully implemented and tested for functionality and user experience.
Verification successful

The ImportModal component is implemented with functionality to import commands and display the results. It uses Vue's composition API, Naive UI components, and custom hooks for managing state and integrations. The component's setup script, template, and styles are correctly defined, indicating a comprehensive implementation aimed at functionality and user experience. However, the effectiveness and absence of bugs can only be fully assessed through testing and user feedback.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Verify the ImportModal component implementation
cat frontend/dashboard/src/components/commands/importModal.vue

Length of output: 2163

* 202-209: The inclusion of the `ImportModal` component within an `NModal` is correctly implemented. Ensure that the modal's size and other properties provide a good user experience on various screen sizes and that the modal closes as expected.
apps/api/internal/impl_protected/integrations/nightbot.go (4)
  • 62-63: The error message "enable nightbot integration first" could be more user-friendly and actionable.

Consider providing a more detailed error message that guides the user on how to enable Nightbot integration, possibly including a link to documentation or settings page.

  • 87-115: The logic to find specific roles (BROADCASTER, MODERATOR, SUBSCRIBER, VIP) involves multiple database queries, which could be optimized.

Consider fetching all roles in a single query and then filtering them in memory to find the specific roles needed. This approach reduces the number of database calls and can improve performance.

  • 145-181: The switch statement for mapping Nightbot user levels to Twir roles is clear, but it lacks support for some user levels and defaults to failure for unsupported levels.

Evaluate if additional mappings between Nightbot user levels and Twir roles are feasible or necessary. If certain levels are intentionally unsupported, consider documenting the rationale within the code or in an external documentation resource.

  • 221-235: Error handling for database operations could be improved by providing more specific feedback based on the error type or code.

Enhance error handling by checking for specific error codes (e.g., unique constraint violations) and returning more informative error messages to the caller. This can help in troubleshooting and user guidance.

libs/api/api.proto (1)
  • 130-134: The addition of Nightbot integration RPC methods is consistent with the existing structure and naming conventions of the API. However, it's important to ensure that the documentation for these methods is clear and comprehensive.

Ensure that each new RPC method has corresponding documentation that explains its purpose, expected inputs, and outputs. This will aid in the maintainability and usability of the API.

Comment on lines 47 to 245
case "admin":
failedCount++
failedCommandsNames = append(
failedCommandsNames,
command.Name+" (command userlevel is not supported)",
)
continue
case "owner":
commandRoles = append(commandRoles, broadcasterRole.ID)
case "moderator":
commandRoles = append(commandRoles, broadcasterRole.ID, moderatorRole.ID)
case "twitch_vip":
commandRoles = append(commandRoles, broadcasterRole.ID, moderatorRole.ID, vipRole.ID)
case "regular":
failedCount++
failedCommandsNames = append(
failedCommandsNames,
command.Name+" (command userlevel is not supported)",
)
continue
case "subscriber":
commandRoles = append(
commandRoles,
broadcasterRole.ID,
moderatorRole.ID,
subscriberRole.ID,
)
case "everyone":
commandRoles = []string{}
case "default":
failedCount++
failedCommandsNames = append(
failedCommandsNames,
command.Name+" (command userlevel is not supported)",
)
}

newCommand := model.ChannelsCommands{
ID: uuid.NewString(),
Name: commandName,
Cooldown: null.IntFrom(int64(command.CoolDown)),
CooldownType: "GLOBAL",
Default: false,
DefaultName: null.String{},
Module: "CUSTOM",
IsReply: true,
KeepResponsesOrder: true,
DeniedUsersIDS: []string{},
AllowedUsersIDS: []string{},
RolesIDS: commandRoles,
OnlineOnly: false,
RequiredWatchTime: 0,
RequiredMessages: 0,
RequiredUsedChannelPoints: 0,
Responses: make(
[]*model.ChannelsCommandsResponses,
0,
1,
),
GroupID: null.String{},
EnabledCategories: pq.StringArray{},
CooldownRolesIDs: pq.StringArray{},
Enabled: true,
Aliases: pq.StringArray{},
Visible: true,
ChannelID: dashboardId,
Description: null.String{},
}

newCommand.Responses = append(newCommand.Responses, &model.ChannelsCommandsResponses{
ID: uuid.NewString(),
Text: null.StringFrom(commandResponse),
Order: 0,
})

err = c.Db.WithContext(ctx).Create(&newCommand).Error
if err != nil {
if pgerr, ok := err.(*pgconn.PgError); ok {
if pgerr.Code == "23505" {
failedCount++
failedCommandsNames = append(
failedCommandsNames,
command.Name+" (command with this name already exists)",
)

continue
}
}

return nil, err
}
importedCount++
}

return &integrations_nightbot.ImportCommandsResponse{
ImportedCount: int32(importedCount),
FailedCount: int32(failedCount),
FailedCommandsNames: failedCommandsNames,
}, nil
}
Copy link

Choose a reason for hiding this comment

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

In the IntegrationsNightbotImportCommands function, there's a potential issue with how user roles are fetched and matched against Nightbot user levels. This process involves multiple database queries and could be optimized. Additionally, the error handling could be improved to provide more specific messages for different failure scenarios.

Consider refactoring the role fetching logic to reduce the number of database queries. For example, fetch all roles at once and then filter them in memory. Also, enhance error messages to be more descriptive about the failure reason.

Comment on lines 66 to 77
commandsData := nightbotCustomCommandsResponse{}
resp, err := req.R().
SetContext(ctx).
SetBearerAuthToken(integration.AccessToken.String).
SetSuccessResult(&commandsData).
Get("https://api.nightbot.tv/1/commands")
if err != nil {
return nil, err
}
if !resp.IsSuccessState() {
return nil, fmt.Errorf("nightbot integration error: %s", resp.String())
}
Copy link

Choose a reason for hiding this comment

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

When making external API calls to Nightbot, there's no explicit timeout set for the request. This could lead to the server waiting indefinitely if Nightbot's API does not respond.

- resp, err := req.R().
+ resp, err := req.R().SetTimeout(10 * time.Second).

Add a timeout to the request to prevent potential hanging of the server process.


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.

Suggested change
commandsData := nightbotCustomCommandsResponse{}
resp, err := req.R().
SetContext(ctx).
SetBearerAuthToken(integration.AccessToken.String).
SetSuccessResult(&commandsData).
Get("https://api.nightbot.tv/1/commands")
if err != nil {
return nil, err
}
if !resp.IsSuccessState() {
return nil, fmt.Errorf("nightbot integration error: %s", resp.String())
}
commandsData := nightbotCustomCommandsResponse{}
resp, err := req.R().
SetTimeout(10 * time.Second).
SetContext(ctx).
SetBearerAuthToken(integration.AccessToken.String).
SetSuccessResult(&commandsData).
Get("https://api.nightbot.tv/1/commands")
if err != nil {
return nil, err
}
if !resp.IsSuccessState() {
return nil, fmt.Errorf("nightbot integration error: %s", resp.String())
}

Comment on lines 170 to 181
</div>
<div>
<n-space>
<n-button
:disabled="!userCanManageCommands" secondary type="info"
@click="showImportModal = true"
>
Import
</n-button>
<n-button
:disabled="!userCanManageCommands" secondary type="info"
@click="showManageGroupsModal = true"
Copy link

Choose a reason for hiding this comment

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

Tip

Codebase Verification

The script output indicates an issue with the condition used to disable the import button in importModal.vue. The condition :disabled="userCanManageCommands && isNightbotIntegrationEnabled" disables the button when the user has the MANAGE_COMMANDS permission and the Nightbot integration is enabled, which seems contrary to the expected behavior. Typically, the button should be enabled when the user has the necessary permissions and the integration is active. This suggests a misunderstanding or error in the implementation of the button's disabled state logic.

Analysis chain

📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [154-178]

The implementation of the import modal trigger and display logic is correct. However, ensure that the button is appropriately enabled or disabled based on the user's permissions and the integration's status. Additionally, verify that the modal provides a good user experience and handles errors gracefully.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Verify user permissions and integration status handling
grep -A 5 "userCanManageCommands" frontend/dashboard/src/components/commands/importModal.vue

Length of output: 523

Copy link

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

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 05d7f1c and fd79bd5.
Files selected for processing (3)
  • apps/api/internal/impl_protected/integrations/nightbot.go (1 hunks)
  • apps/integrations/src/index.js (1 hunks)
  • frontend/dashboard/src/components/integrations/nightbot.vue (1 hunks)
Files skipped from review as they are similar to previous changes (3)
  • apps/api/internal/impl_protected/integrations/nightbot.go
  • apps/integrations/src/index.js
  • frontend/dashboard/src/components/integrations/nightbot.vue

Copy link

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

Review Status

Actionable comments generated: 3

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between fd79bd5 and 4ba22fb.
Files selected for processing (6)
  • apps/api/internal/impl_protected/integrations/nightbot.go (1 hunks)
  • frontend/dashboard/src/api/integrations/nightbot.ts (1 hunks)
  • frontend/dashboard/src/components/timers/importModal.vue (1 hunks)
  • frontend/dashboard/src/pages/Timers.vue (3 hunks)
  • libs/api/api.proto (2 hunks)
  • libs/api/messages/integrations_nightbot/integrations_nightbot.proto (1 hunks)
Files skipped from review as they are similar to previous changes (3)
  • apps/api/internal/impl_protected/integrations/nightbot.go
  • frontend/dashboard/src/api/integrations/nightbot.ts
  • libs/api/messages/integrations_nightbot/integrations_nightbot.proto
Additional comments (5)
frontend/dashboard/src/components/timers/importModal.vue (2)
  • 3-3: Consider adding error handling for the import operation to provide feedback to the user in case the import fails.
  • 47-56: Ensure that res is properly initialized and updated to reflect the actual response structure from the import operation. It seems like res is expected to have importedCount, failedCount, and failedTimersNames properties, but there's no validation or type assertion in the script section. Consider adding checks or using TypeScript interfaces to ensure these properties exist and are used correctly.
frontend/dashboard/src/pages/Timers.vue (2)
  • 124-124: The showImportModal ref is correctly added to manage the visibility of the import modal. Ensure that all related logic correctly toggles this ref to show and hide the modal as expected.
  • 165-175: The modal configuration for the import modal is well-defined. However, consider adding accessibility attributes such as aria-labelledby and aria-describedby to improve accessibility for screen reader users.
libs/api/api.proto (1)
  • 130-135: The new RPC methods for Nightbot integration are correctly defined. Ensure that the corresponding service implementations handle errors gracefully and provide meaningful error messages to the clients, especially for operations like importing commands and timers which might fail due to external API limitations or data inconsistencies.

Comment on lines 31 to 32
const isNightbotIntegrationEnabled = computed(() => {
return !data.value?.userName;
Copy link

Choose a reason for hiding this comment

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

The computed property isNightbotIntegrationEnabled seems to be incorrectly named or implemented. It checks if userName is not present, which might indicate that the integration is not enabled. However, the name suggests the opposite. Consider renaming this property or adjusting its logic to accurately reflect its purpose.

</div>

<template #footer>
<n-button secondary type="success" :disabled="userCanManageTimers && isNightbotIntegrationEnabled" :loading="nightbotTimersImporter.isLoading.value" @click="onImportClick">
Copy link

Choose a reason for hiding this comment

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

The condition for disabling the import button seems incorrect. It should likely be !userCanManageTimers || !isNightbotIntegrationEnabled to disable the button when the user cannot manage timers or when the Nightbot integration is not enabled.

- :disabled="userCanManageTimers && isNightbotIntegrationEnabled"
+ :disabled="!userCanManageTimers || !isNightbotIntegrationEnabled"

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.

Suggested change
<n-button secondary type="success" :disabled="userCanManageTimers && isNightbotIntegrationEnabled" :loading="nightbotTimersImporter.isLoading.value" @click="onImportClick">
<n-button secondary type="success" :disabled="!userCanManageTimers || !isNightbotIntegrationEnabled" :loading="nightbotTimersImporter.isLoading.value" @click="onImportClick">

</n-button>
<div>
<n-button
:disabled="!userCanManageTimers" secondary type="info"
Copy link

Choose a reason for hiding this comment

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

The :disabled binding on the import button should ensure that the button is disabled when the user does not have the MANAGE_TIMERS permission. However, it's missing .value to correctly evaluate the computed property. Update it to :disabled="!userCanManageTimers.value".

- :disabled="!userCanManageTimers" secondary type="info"
+ :disabled="!userCanManageTimers.value" secondary type="info"

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.

Suggested change
:disabled="!userCanManageTimers" secondary type="info"
:disabled="!userCanManageTimers.value" secondary type="info"

Copy link

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

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 4ba22fb and 1e929bf.
Files selected for processing (2)
  • frontend/dashboard/src/components/commands/importModal.vue (1 hunks)
  • frontend/dashboard/src/components/timers/importModal.vue (1 hunks)
Files skipped from review as they are similar to previous changes (2)
  • frontend/dashboard/src/components/commands/importModal.vue
  • frontend/dashboard/src/components/timers/importModal.vue

@danluki danluki changed the title feat: nightbot commands import feat: nightbot import Mar 28, 2024
@Satont Satont changed the base branch from main to feat/nightbot-import March 29, 2024 02:13
@Satont Satont merged commit 79fc258 into twirapp:feat/nightbot-import Mar 29, 2024
Satont added a commit that referenced this pull request Mar 29, 2024
* feat: nightbot import (#652)

* feat: nightbot integration migration

* feat: nightbot integration backend

* feat: nightbot integration frontend

* chore: miss import

* chore: some fixes

* feat: nightbot timers import, buf fixes, tokens refreshing

* chore: invalid permissions

* refactor(dashboard): move everything to separate page

* fix(dashboard): add missed requires props

---------

Co-authored-by: Danil Lukinykh <[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.

2 participants