Skip to content

Feat/remove roman related code wpb 17568 #18

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

Jump to bottom
Merged
olanokhin merged 18 commits into from
May 27, 2025
Merged

Feat/remove roman related code wpb 17568 #18

olanokhin merged 18 commits into from
May 27, 2025

Conversation

@olanokhin
Copy link

@olanokhin olanokhin commented May 23, 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

  • Improved recurring reminder display and cron interpretation.
  • Enhanced reminder listing with timestamps.
  • Fixed serialization/deserialization of QualifiedId.
  • Updated SQL schema for longer conversation IDs.
  • Removed obsolete code and dependencies.
  • Improved code quality (built with ktlint, detekt).
  • Added and improved unit test coverage.

Causes (Optional)

  • Scheduled time for /remind list was not displayed at all.
  • Reminder IDs and conversation IDs were inconsistently serialized, causing issues with job scheduling and persistence.
  • Outdated or unused code (e.g., PlainId, Token handling) cluttered the codebase.
  • SQL schema limited conversation ID length, causing potential data issues use obsolete tables schema.

Solutions

  • Updated output messages logic with scheduled time and "ready to copy" formatting.
  • Implemented CronInterpreter to handle custom cron patterns and improved output for recurring reminders.
  • Added QualifiedIdExtension and QualifiedIdConverter for robust QualifiedId serialization/deserialization.
  • Updated SQL schema to support 255-character conversation IDs.
  • Removed unused code, dependencies SQL tables.
  • Improved test coverage for cron and QualifiedId utilities.
  • Clean code style with ktlint and detekt.

Dependencies (Optional)

  • WireSDK 0.0.7
  • google.protobuf 4.31.0

Testing

Test Coverage (Optional)

  • I have added automated test to this contribution

How to Test

  1. Add credentials in demo.properties
  2. Run the db with docker compose up db
  3. Build the App with ./gradlew clean build -Dquarkus.package.jar.type=uber-jar
  4. Run the app with java -jar build/reminders-bot-1.0.0-SNAPSHOT-runner.jar
  5. Login to chala.wire.link with your account
  6. Create new conversation
  7. Add Remind-App to conversation
  8. Send /remind to "Do smth" "every monday at 09:00".
  9. Send /remind list to see active reminder full info.
  10. Send /remind delete <reminder UUID> to delete any type of reminder by UUID provided with /remind list command.

Notes (Optional)

  • The app is still in development; see README for current limitations and planned improvements.
  • Obsolete token and plain ID handling have been removed and replaced by QualifiedId to fit WireSDK.

olanokhin added 9 commits May 22, 2025 23:19
@olanokhin
Disabled: ktlint, detekt, Token handling;
50a92a5 
app builds and reminder logic works properly;
@olanokhin
Delete: Token repository, signal handling;
fa2ffd5
@olanokhin
Update: WireSDK to 0.0.7, google.protobuf to 4.31.0;
ab46ae0 
Delete: Quarkus rest client dependencies;
Add timestamp to reminders on list command;
Create interpretCron() to properly display recurrent reminder timestamps;
@olanokhin
Replace: PlainConversationId by QualifiedId;
79df871 
Create: QualifiedIdConverter for serialization/deserialization;
Add: TEST_CONVERSATION_ID to tests for mocking QualifiedId;
Fix: givenTomorrowIsGiven_thenTheDateTimeShouldMatchForNowPlusOneDay to use same time reference (UTC);
@olanokhin
Update: SQL Tables schema to handle 255 chars for conversationId;
3f879f3 
Add: QualifiedIdExtension to encapsulate QualifiedId serialisation/deserialization;
Fix: ReminderJob repository to use full UUID instead obfuscated with .toRawString();
Delete: Tokens table, V3 migration with domain column;
@olanokhin
Refactor: fn interpretCron() to object CronInterpret in utils;
dd0cbbb 
Delete: PlainId.kt;
Update: taskId from :TaskId:String to :String;
Fix: ktlint and detekt issues;
@olanokhin
Add: unit tests for CronInterpreter and QualifiedIdExtension;
58a951d 
Fix: CronInterpreter to return custom cron on unhandled pattern;
@olanokhin
Fix: ktlint issues;
861e108
@olanokhin
Update: readme with latest features and fixes;
339bf7e 
Add: list of future updates for next PRs;
@olanokhin olanokhin requested a review from a team as a code owner May 23, 2025 17:36
@olanokhin olanokhin self-assigned this May 23, 2025
@olanokhin olanokhin added bug Something isn't working enhancement New feature or request dependencies Pull requests that update a dependency file labels May 23, 2025
MarianKijewski
MarianKijewski reviewed May 26, 2025
View reviewed changes
spoonman01
spoonman01 requested changes May 26, 2025
View reviewed changes
@olanokhin olanokhin merged commit da4dde3 into main May 27, 2025
2 checks passed
@olanokhin olanokhin deleted the feat/Remove-Roman-related-code-WPB-17568 branch May 27, 2025 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MarianKijewski MarianKijewski MarianKijewski left review comments

@spoonman01 spoonman01 spoonman01 approved these changes

Assignees

@olanokhin olanokhin

Labels

bug Something isn't working dependencies Pull requests that update a dependency file enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@olanokhin @spoonman01 @MarianKijewski