Skip to content

Conversation

@olanokhin
Copy link

@olanokhin olanokhin commented Sep 18, 2025


PR Submission Checklist for internal contributors

  • The PR Title

    • conforms to the style of semantic commits messages¹ supported in Wire's Github Workflow²
    • contains a reference JIRA issue number like SQPIT-764
    • answers the question: If merged, this PR will: ... ³
  • The PR Description

    • is free of optional paragraphs and you have filled the relevant parts to the best of your ability

What's new in this PR?

Issues

  • The bot did not support interactive actions, such as deleting a reminder by clicking a button.
  • Creating recurring reminders for "every day" would fail with a ParseException due to an invalid cron string format.
  • A race condition could cause duplicate key errors when a user retried a failed reminder creation command.
  • The /remind list command displayed incorrect text for daily reminders (e.g., "every month" instead of "every day").
  • Several code quality warnings were present, including swallowed exceptions and unused variables.

Causes (Optional)

  • The application lacked handlers for BUTTON_ACTION events from the messaging platform.
  • The CronInterpreter generated cron strings for "every day" (* * *) that were incompatible with the Quartz scheduler, which requires one of the day fields (day-of-month or day-of-week) to be ?.
  • The logic to convert cron strings back to text (cronToText) was incorrectly identifying the pattern for "every day".
  • In the handleNewReminder flow, the reminder was saved to the database before the confirmation message was generated. If message generation failed (due to the invalid cron), an exception was thrown, but the saved data remained, leading to a duplicate key error on retry.
  • Exception handling in some parts of the code was too broad or swallowed exceptions, hiding the root cause of failures.

Solutions

  • Button Action for Deletion: Implemented a full flow to handle BUTTON_ACTION events. When a "Delete" button is clicked, the bot now correctly identifies the reminder, deletes it, sends a ButtonActionConfirmation to update the original message's button state, and sends a new message confirming the deletion. This also works correctly if the reminder has already been deleted by another action.
  • Cron Handling Fixes:
    • Modified CronInterpreter to generate Quartz-compatible cron strings (e.g., ? * * for "every day").
    • Updated the cronToText logic to correctly parse and display daily recurring reminders.
  • Transactional Reminder Creation: Refactored CommandHandler.handleNewReminder to first validate all inputs and generate the confirmation message. The reminder is now saved to the database only if all preceding steps succeed. This prevents inconsistent states and resolves the duplicate key error.
  • Code Quality Improvements:
    • Replaced try-catch blocks for control flow with explicit checks (e.g., using a regex for UUID validation) to resolve SwallowedException warnings.
    • Removed exception swallowing in Reminder.kt to ensure cron parsing errors propagate correctly.
    • Replaced for loops with repeat where the index was unused.
  • Unit Tests: Updated and corrected unit tests for CronInterpreter to align with the new, valid cron formats.
  • Dependency Updates: Updated Gradle, Quarkus, Wire SDK, and other dependencies to their latest versions to ensure compatibility and security.

Dependencies (Optional)

  • No new dependencies were added.

Testing

Test Coverage (Optional)

  • I have added automated test to this contribution

How to Test

  1. Create a recurring reminder:
    /remind to "Daily Standup" "every day at 10:00"
    
    • Verify that a confirmation message is received with the correct next 5 schedules.
  2. List reminders:
    /remind list
    
    • Verify that the new reminder is listed with the text 'Daily Standup' at: every day at 10:00.
    • Verify that a "Delete" button is present.
  3. Delete reminder via button:
    • Click the "Delete" button on the message from the list command.
    • Verify that the button is updated/disabled.
    • Verify that a new message is received confirming the deletion (e.g., "The reminder 'Daily Standup' was deleted.").
  4. Attempt to delete again:
    • Click the "Delete" button on creation confirmation message.
    • Verify the button is updated and a message is received indicating the reminder was not found.

PR Post Submission Checklist for internal contributors (Optional)

  • Wire's Github Workflow has automatically linked the PR to a JIRA issue

References
  1. https://sparkbox.com/foundry/semantic_commit_messages
  2. https://github.com/wireapp/.github#usage
  3. E.g. feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764.

Update: sdk password to fit latest 0.0.16 version (32 chars)
WIRE_SDK_API_URL,WIRE_SDK_API_BOT_KEY;
…arate composite messages where delete button has same UUID as a reminder;
Create: separate DTO for new Message and new ButtonAction;
Split: process logic;
Update: delete reminder, to listen button actions and send button action confirmation;
@olanokhin olanokhin self-assigned this Sep 18, 2025
@olanokhin olanokhin requested a review from a team as a code owner September 18, 2025 15:18
@olanokhin olanokhin added enhancement New feature or request dependencies Pull requests that update a dependency file labels Sep 18, 2025
… messages in separate helper functions to make core methods of Command handler easeir to read and understand;
Copy link

@bbaarriiss bbaarriiss left a comment

Choose a reason for hiding this comment

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

LGTM thanks @olanokhin . But I would like you to wait also for @yamilmedina 's review.

Copy link

@yamilmedina yamilmedina left a comment

Choose a reason for hiding this comment

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

lgtm 🚀

@olanokhin olanokhin merged commit e2dca68 into main Sep 19, 2025
3 checks passed
@alexandreferris alexandreferris deleted the feat/Use-Composite-messages-to-improve-UX-WPB-17856 branch September 19, 2025 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants