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.

Sign up for GitHub

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(github): Modal for GitHub issue #73

Merged
merged 2 commits into from
Mar 3, 2024
Merged

feat(github): Modal for GitHub issue #73

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5216b92
feat(github): Modal for GitHub issue
Mar 2, 2024
ea1645c
fix: update modal submission wrapping
JacobCoffee Mar 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 83 additions & 25 deletions src/byte/plugins/custom/litestar.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,28 +1,105 @@
"""Custom plugins for the Litestar Discord."""
from __future__ import annotations

from discord import Embed, Interaction, Message, app_commands
from typing import Self

from discord import Embed, Interaction, Message, TextStyle, app_commands
from discord.ext.commands import Bot, Cog, Context, command, group, is_owner
from discord.ui import Modal, TextInput
from discord.utils import MISSING
from httpx import codes

from byte.lib.utils import is_byte_dev, mention_role, mention_user
from server.domain.github.helpers import github_client

__all__ = ("LitestarCommands", "setup")


class GitHubIssue(Modal, title="Create GitHub Issue"):
Copy link
Contributor

Choose a reason for hiding this comment

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

issue (llm): While the introduction of a GitHubIssue modal class leverages Discord's UI features for a more interactive experience, it significantly increases the complexity of the code. This is due to the addition of new concepts such as Modal, TextInput, and TextStyle, along with more sophisticated error handling and asynchronous logic. Additionally, the increased number of dependencies and the custom initialization logic in the GitHubIssue class add further complexity.

To maintain simplicity and ease of maintenance, consider keeping the interaction logic within a function instead of introducing a modal class. This approach can reduce the cognitive load for developers and minimize the potential for bugs, while still achieving the goal of creating GitHub issues from Discord messages. Simplifying the code in this manner can make it more accessible to other developers and easier to maintain in the long run.

Copy link
Contributor

Choose a reason for hiding this comment

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

issue (llm): I noticed the new GitHubIssue modal class has been introduced, but there are no unit tests accompanying this addition. It's crucial to ensure that the modal's behavior, especially the on_submit method, works as expected under various conditions. Could you add unit tests to cover the initialization of this modal, its submission behavior, and error handling? This would include testing the successful creation of a GitHub issue as well as failure scenarios, such as API errors.

"""Modal for GitHub issue creation."""

title_ = TextInput[Self](label="title", placeholder="Title")
description = TextInput[Self](
label="Description",
style=TextStyle.paragraph,
placeholder="Please enter an description of the bug you are encountering.",
)
mcve = TextInput[Self](
label="MCVE",
style=TextStyle.paragraph,
placeholder="Please provide a minimal, complete, and verifiable example of the issue.",
)
logs = TextInput[Self](
label="Logs", style=TextStyle.paragraph, placeholder="Please copy and paste any relevant log output."
)
version = TextInput[Self](
Comment on lines +21 to +35
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion (llm): The renaming of the title attribute to title_ to avoid conflict is a pragmatic solution. However, it's worth considering if there could be a more elegant approach to handle such conflicts, perhaps by namespacing or using a different attribute naming convention that reduces the likelihood of such conflicts in the first place.

label="Litestar Version", placeholder="What version of Litestar are you using when encountering this issue?"
)

def __init__(
self,
*,
title: str = MISSING,
timeout: float | None = None,
custom_id: str = MISSING,
message: Message | None = None,
) -> None:
# NOTE: check how else to set default
super().__init__(title=title, timeout=timeout, custom_id=custom_id)
if message:
self.description.default = message.content

async def on_submit(self, interaction: Interaction) -> None:
issue_reporter = interaction.user
issue_body_lines = [
"### Reported by",
f"[{issue_reporter.display_name}](https://discord.com/users/{issue_reporter.id}) in Discord: {interaction.channel.name}", # noqa: E501
"",
"### Description",
f"{self.description.value.strip()}",
"",
"### MCVE",
f"{self.mcve.value.strip()}",
"",
"### Logs",
f"{self.logs.value.strip()}",
"",
"### Litestar Version",
f"{self.version.value.strip()}",
]
issue_body = "\n".join(issue_body_lines)
try:
response_wrapper = await github_client.rest.issues.async_create(
owner="litestar-org", repo="litestar", data={"title": self.title_.value, "body": issue_body}
)
if codes.is_success(response_wrapper.status_code):
await interaction.response.send_message(
f"GitHub Issue created: {response_wrapper.parsed_data.html_url}", ephemeral=False
)
else:
await interaction.response.send_message("Issue creation failed.", ephemeral=True)

except Exception as e: # noqa: BLE001
await interaction.response.send_message(f"An error occurred: {e!s}", ephemeral=True)


class LitestarCommands(Cog):
"""Litestar command cog."""

def __init__(self, bot: Bot) -> None:
"""Initialize cog."""
self.bot = bot
self.__cog_name__ = "Litestar Commands" # type: ignore[misc]
self.context_menu = app_commands.ContextMenu(name="Create GitHub Issue", callback=self.create_github_issue)
self.context_menu = app_commands.ContextMenu(
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion (llm): It's great to see the context menu being updated to trigger the new GitHub issue modal. However, I didn't find any integration tests that simulate the user interaction with this context menu and subsequently the modal. Testing this end-to-end flow is important to ensure that the modal is correctly invoked and that the interaction flow works as intended. Could you add such integration tests?

# TODO: Name changed to not conflict with the other one, discord shows both
name="Create GitHub Issue",
callback=self.create_github_issue_modal,
)
bot.tree.add_command(self.context_menu)

@group(name="litestar")
@is_byte_dev()
async def litestar(self, ctx: Context) -> None:
async def litestar(self, ctx: Context[Bot]) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

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

question (llm): I see that the litestar command group and its subcommands have been modified. While this change might not directly relate to testing the GitHub issue modal, it's important to ensure that these modifications do not inadvertently affect the functionality of the commands. If not already done, could you ensure that there are adequate tests for these command group changes, including any new behavior or edge cases introduced?

"""Commands for the Litestar guild."""
if ctx.invoked_subcommand is None:
await ctx.send("Invalid Litestar command passed...")
Expand All @@ -35,7 +112,7 @@ async def litestar(self, ctx: Context) -> None:
hidden=True,
)
@is_owner()
async def apply_role_embed(self, ctx: Context) -> None:
async def apply_role_embed(self, ctx: Context[Bot]) -> None:
"""Apply the role information embed to a channel.
Args:
Expand Down Expand Up @@ -87,33 +164,14 @@ async def apply_role_embed(self, ctx: Context) -> None:

await ctx.send(embed=embed)

async def create_github_issue(self, interaction: Interaction, message: Message) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

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

issue (llm): The removal of the create_github_issue method and its replacement with create_github_issue_modal is a significant change. It's essential to verify through tests that the new modal-based approach achieves the same end goal as the previous implementation. Specifically, tests should cover the modal's successful submission leading to a GitHub issue creation and handle any potential errors gracefully. If such tests are missing, please consider adding them to ensure the new implementation's reliability.

async def create_github_issue_modal(self, interaction: Interaction, message: Message) -> None:
"""Context menu command to create a GitHub issue from a Discord message.
Args:
interaction: Interaction object.
message: Message object.
"""
issue_title = "Issue from Discord"
issue_reporter = message.author
issue_body = (
f"Reported by {issue_reporter.display_name} in Discord: {message.channel.mention}:\n\n{message.content}"
)

try:
response_wrapper = await github_client.rest.issues.async_create(
owner="litestar-org", repo="litestar", data={"title": issue_title, "body": issue_body}
)

if response_wrapper._response.is_success:
issue_data = response_wrapper._data_model.parse_obj(response_wrapper._response.json())
issue_url = issue_data.html_url
await interaction.response.send_message(f"GitHub Issue created: {issue_url}", ephemeral=False)
else:
await interaction.response.send_message("Issue creation failed.", ephemeral=True)

except Exception as e: # noqa: BLE001
await interaction.response.send_message(f"An error occurred: {e!s}", ephemeral=True)
await interaction.response.send_modal(GitHubIssue(message=message))


async def setup(bot: Bot) -> None:
Expand Down
Loading