feature/capr 59 service layer profile#162
Conversation
Reviewer's GuideRefactors the profile cog to delegate persistence, validation, and embed construction to a new ProfileService, introduces safer button/view wiring for the profile modal launcher, and expands tests to cover the new service-driven behavior and error conditions. Sequence diagram for service-based profile create and update flowsequenceDiagram
actor User
participant Discord as DiscordClient
participant Profile as ProfileCog
participant Service as ProfileService
participant Launcher as ProfileModalLauncherView
User->>Discord: invoke /profile action=create|update
Discord->>Profile: profile(interaction, action)
Profile->>Profile: handle_edit_action(interaction, action)
Profile->>Service: start_edit(user.id, action)
alt profile exists for create
Service-->>Profile: raise ProfileExistsError
Profile->>Discord: _send_error("Profile Exists")
else profile missing for update
Service-->>Profile: raise ProfileNotFoundError
Profile->>Discord: _send_error("No Profile")
else valid start_edit
Service-->>Profile: initial_data
Profile->>Profile: _open_profile_identity_modal(interaction, action, initial_data)
Profile->>Discord: show identity modal
User->>Discord: submit identity
Discord->>Profile: _handle_profile_identity_submit(interaction, identity, action)
Profile->>Service: merge_identity_step(user.id, identity)
Service-->>Profile: profile_data
Profile->>Launcher: create ProfileModalLauncherView(callback=_open_profile_details_modal)
Profile->>Discord: send button view
User->>Discord: click profile button
Discord->>Launcher: ProfileLaunchButton.callback(interaction)
Launcher->>Launcher: open_modal(interaction)
Launcher->>Profile: _open_profile_details_modal(interaction, action, profile_data)
Profile->>Discord: show details modal
User->>Discord: submit details
Discord->>Profile: _handle_profile_details_submit(interaction, details, profile_data, action)
Profile->>Service: finalize_profile(user, details, profile_data, action)
alt invalid combined profile
Service-->>Profile: raise InvalidProfileError
Profile->>Discord: _send_error("Profile Validation Failed")
else valid combined profile
Service-->>Profile: profile, result(created|updated)
Profile->>Profile: _handle_profile_submit(interaction, profile, result)
Profile->>Service: create_profile_embed(user, profile)
Service-->>Profile: embed
Profile->>Discord: send success + profile embeds
end
end
Class diagram for new ProfileService and updated profile cogclassDiagram
class ProfileService {
-logging.Logger log
-dict~int,UserProfileSchema~ _profiles
+ProfileService(bot: discord.Client, log: logging.Logger)
+start_edit(user_id: int, action: str) dict~str,Any~
+merge_identity_step(user_id: int, identity: UserProfileIdentitySchema) dict~str,Any~
+build_profile(user: discord.abc.User, details: UserProfileDetailsSchema, profile_data: dict~str,Any~) UserProfileSchema
+finalize_profile(user: discord.abc.User, details: UserProfileDetailsSchema, profile_data: dict~str,Any~, action: Literal_create_update_) tuple~UserProfileSchema,Literal_created_updated_~
+get_profile(user_id: int) UserProfileSchema
+save_profile(user: discord.abc.User, profile: UserProfileSchema) void
+delete_profile(user: discord.abc.User) void
+create_profile_embed(user: discord.User|discord.Member, profile: UserProfileSchema) discord.Embed
}
class ProfileExistsError {
}
class ProfileNotFoundError {
}
class InvalidProfileError {
}
class Profile {
-commands.Bot bot
-logging.Logger log
-ProfileService service
+handle_edit_action(interaction: discord.Interaction, action: str) void
+_handle_profile_identity_submit(interaction: discord.Interaction, identity: UserProfileIdentitySchema, action: str) void
+_open_profile_details_modal(interaction: discord.Interaction, action: str, profile_data: dict~str,Any~) void
+_handle_profile_details_submit(interaction: discord.Interaction, details: UserProfileDetailsSchema, profile_data: dict~str,Any~, action: Literal_create_update_) void
+handle_show_action(interaction: discord.Interaction) void
+handle_delete_action(interaction: discord.Interaction) void
+_handle_profile_submit(interaction: discord.Interaction, profile: UserProfileSchema, result: Literal_created_updated_) void
+_send_error(interaction: discord.Interaction, title: str, message: str) void
}
class ConfirmDeleteView {
+bool|None value
+ConfirmDeleteView()
+confirm(interaction: discord.Interaction, button: ui.Button) void
+cancel(interaction: discord.Interaction, button: ui.Button) void
}
class ProfileModalLauncherView {
-Callable callback
+ProfileModalLauncherView(callback: Callable, button_label: str, button_emoji: str|None, button_style: discord.ButtonStyle)
+open_modal(interaction: discord.Interaction) void
}
class ProfileLaunchButton {
+launcher_view ProfileModalLauncherView
+callback(interaction: discord.Interaction) void
}
class BaseView {
}
class UIButton {
}
ProfileService ..> UserProfileSchema
ProfileService ..> UserProfileIdentitySchema
ProfileService ..> UserProfileDetailsSchema
ProfileService ..> ProfileExistsError
ProfileService ..> ProfileNotFoundError
ProfileService ..> InvalidProfileError
Profile --> ProfileService
ConfirmDeleteView --|> BaseView
ProfileModalLauncherView --|> BaseView
ProfileLaunchButton --|> UIButton
ProfileModalLauncherView o--> ProfileLaunchButton
ProfileLaunchButton --> ProfileModalLauncherView
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Tests currently reach into
cog.service._profilesto set up state; consider exposing a small public helper (e.g.set_profileor usingsave_profile) to avoid coupling tests to the service’s internal storage details. ProfileService.start_edittakesaction: strwhilefinalize_profileusesLiteral["create", "update"]; tighteningstart_editto the sameLiteral(and possibly validating inputs centrally) would make the API safer and more self-documenting.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Tests currently reach into `cog.service._profiles` to set up state; consider exposing a small public helper (e.g. `set_profile` or using `save_profile`) to avoid coupling tests to the service’s internal storage details.
- `ProfileService.start_edit` takes `action: str` while `finalize_profile` uses `Literal["create", "update"]`; tightening `start_edit` to the same `Literal` (and possibly validating inputs centrally) would make the API safer and more self-documenting.
## Individual Comments
### Comment 1
<location path="tests/capy_discord/exts/test_profile.py" line_range="48-57" />
<code_context>
+
+
+@pytest.mark.asyncio
+async def test_profile_create_with_existing_profile_shows_error(cog, interaction):
+ cog.service._profiles[interaction.user.id] = UserProfileSchema(
+ preferred_name="Existing User",
+ student_id="123456789",
+ school_email="existing@school.edu",
+ graduation_year=2028,
+ major="CS",
+ minor="ITWS",
+ description="Already here",
+ )
+
+ await cog.profile.callback(cog, interaction, "create")
+
+ interaction.response.send_modal.assert_not_called()
+ interaction.response.send_message.assert_called_once()
+ embed = interaction.response.send_message.await_args.kwargs["embed"]
+ assert embed.title == "Profile Exists"
+
+
</code_context>
<issue_to_address>
**nitpick (testing):** Avoid reaching into the service's internal _profiles dict directly in tests
The tests seed state via `cog.service._profiles[...]`, which tightly couples them to `ProfileService`’s internal storage. To keep tests resilient to refactors, prefer seeding via a public method (e.g. `save_profile`) or by using the handler flow to create a profile where feasible. This keeps tests behaviour‑focused and avoids breakage if the storage implementation changes.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| async def test_profile_create_with_existing_profile_shows_error(cog, interaction): | ||
| cog.service._profiles[interaction.user.id] = UserProfileSchema( | ||
| preferred_name="Existing User", | ||
| student_id="123456789", | ||
| school_email="existing@school.edu", | ||
| graduation_year=2028, | ||
| major="CS", | ||
| minor="ITWS", | ||
| description="Already here", | ||
| ) |
There was a problem hiding this comment.
nitpick (testing): Avoid reaching into the service's internal _profiles dict directly in tests
The tests seed state via cog.service._profiles[...], which tightly couples them to ProfileService’s internal storage. To keep tests resilient to refactors, prefer seeding via a public method (e.g. save_profile) or by using the handler flow to create a profile where feasible. This keeps tests behaviour‑focused and avoids breakage if the storage implementation changes.
Summary by Sourcery
Extract profile storage, validation, and embed construction into a dedicated service layer and update the profile cog to use it while expanding test coverage of profile flows.
New Features:
Enhancements:
Tests: