-
-
Notifications
You must be signed in to change notification settings - Fork 43
feat(snippets): add member filtering to list snippets command #1205
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,16 +5,20 @@ | |
| all available code snippets in Discord guilds. | ||
| """ | ||
|
|
||
| import discord | ||
| from discord.ext import commands | ||
| from reactionmenu import ViewButton, ViewMenu | ||
| from sqlalchemy import desc | ||
|
|
||
| from tux.core.bot import Tux | ||
| from tux.core.converters import FlexibleUserConverter | ||
| from tux.database.models import Snippet | ||
| from tux.shared.constants import SNIPPET_PAGINATION_LIMIT | ||
|
|
||
| from . import SnippetsBaseCog | ||
|
|
||
| _MEMBER_PARAM = commands.param(default=None, converter=FlexibleUserConverter()) | ||
|
|
||
|
|
||
| class ListSnippets(SnippetsBaseCog): | ||
| """Discord cog for listing snippets.""" | ||
|
|
@@ -37,33 +41,39 @@ def __init__(self, bot: Tux) -> None: | |
| async def list_snippets( | ||
| self, | ||
| ctx: commands.Context[Tux], | ||
| member: discord.User | None = _MEMBER_PARAM, | ||
| *, | ||
| search_query: str | None = None, | ||
|
Comment on lines
41
to
46
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: When Suggested FixModify the command's signature to ensure that text not matching a user is passed to the Prompt for AI AgentDid we get this right? 👍 / 👎 to inform future reviews. |
||
| ) -> None: | ||
| """List snippets, optionally filtering by a search query. | ||
| """List snippets, optionally filtering by member or search query. | ||
|
|
||
| Displays snippets in a paginated embed, sorted by usage count (descending). | ||
| The search query filters by snippet name or content (case-insensitive). | ||
| Optionally filter by a member's snippets or a search query. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| ctx : commands.Context[Tux] | ||
| The context of the command. | ||
| member : discord.User | None, optional | ||
| The member whose snippets to list. | ||
| search_query : str | None, optional | ||
| The query to filter snippets by name or content. | ||
| """ | ||
| assert ctx.guild | ||
|
|
||
| # Fetch snippets with database-level ordering and optional search | ||
| if search_query: | ||
| if member is not None: | ||
| filtered_snippets = await self.db.snippet.get_snippets_by_creator( | ||
| member.id, | ||
| ctx.guild.id, | ||
| ) | ||
| filtered_snippets.sort(key=lambda s: s.uses, reverse=True) | ||
|
Comment on lines
+65
to
+69
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (performance): Consider pushing the ordering by usage count into the DB for the member-filtered query as well. In Suggested implementation: if member is not None:
filtered_snippets = await self.db.snippet.get_snippets_by_creator(
member.id,
ctx.guild.id,
order_by=desc(Snippet.__table__.c.uses), # type: ignore[attr-defined]
)
elif search_query:To make this compile and work correctly, you'll also need to:
|
||
| elif search_query: | ||
| filtered_snippets = await self.db.snippet.search_snippets( | ||
| ctx.guild.id, | ||
| search_query, | ||
| ) | ||
| # Sort search results by usage count (most used first) | ||
| filtered_snippets.sort(key=lambda s: s.uses, reverse=True) | ||
| else: | ||
| # Fetch all snippets ordered by usage count from database | ||
| filtered_snippets = await self.db.snippet.get_all_snippets_by_guild_id( | ||
| ctx.guild.id, | ||
| order_by=desc(Snippet.__table__.c.uses), # type: ignore[attr-defined] | ||
|
|
@@ -72,7 +82,9 @@ async def list_snippets( | |
| if not filtered_snippets: | ||
| await self.send_snippet_error( | ||
| ctx, | ||
| description="No snippets found matching your query." | ||
| description=f"No snippets found for {member.display_name}." | ||
| if member | ||
| else "No snippets found matching your query." | ||
| if search_query | ||
| else "No snippets found.", | ||
| ) | ||
|
|
@@ -81,7 +93,6 @@ async def list_snippets( | |
| # Set up pagination menu | ||
| menu = ViewMenu(ctx, menu_type=ViewMenu.TypeEmbed, show_page_director=False) | ||
|
|
||
| # Add pages based on filtered snippets | ||
| total_snippets = len(filtered_snippets) | ||
|
|
||
| for i in range(0, total_snippets, SNIPPET_PAGINATION_LIMIT): | ||
|
|
@@ -92,6 +103,7 @@ async def list_snippets( | |
| page_snippets, | ||
| total_snippets, | ||
| search_query, | ||
| member, | ||
| ) | ||
|
|
||
| menu.add_page(embed) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: The new positional
memberconverter prevents text queries from falling back to search: non-user first tokens now error out instead of running snippet search.Prompt for AI agents