-
Notifications
You must be signed in to change notification settings - Fork 3
Add support for ZonedDateTime in EventHandler
#136
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
Add support for ZonedDateTime in EventHandler
#136
Conversation
Signed-off-by: Arnau Mora <arnyminerz@proton.me>
Signed-off-by: Arnau Mora <arnyminerz@proton.me>
rfc2822
left a comment
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.
See #135 (comment)
The "parse and catch exception" approach doesn't seem to be a proper solution, at least not in the handle().
I think we should define a separate parseStartDate() method that
- has well-defined inputs and outputs,
- supports the required input formats,
- is tested separately.
Within that method, we should use other methods to determine the input format. If that's really not possible, we can still use the exception method.
lib/src/main/kotlin/at/bitfire/vcard4android/contactrow/EventHandler.kt
Outdated
Show resolved
Hide resolved
lib/src/test/kotlin/at/bitfire/vcard4android/contactrow/EventHandlerTest.kt
Show resolved
Hide resolved
Signed-off-by: Arnau Mora <arnyminerz@proton.me>
|
Should be ready. I've added tests for all formats from AOSP Contacts which should cover the original issue (bitfireAT/davx5-ose#1797 - |
lib/src/main/kotlin/at/bitfire/vcard4android/contactrow/EventHandler.kt
Outdated
Show resolved
Hide resolved
lib/src/main/kotlin/at/bitfire/vcard4android/contactrow/EventHandler.kt
Outdated
Show resolved
Hide resolved
lib/src/main/kotlin/at/bitfire/vcard4android/contactrow/EventHandler.kt
Outdated
Show resolved
Hide resolved
lib/src/main/kotlin/at/bitfire/vcard4android/contactrow/EventHandler.kt
Outdated
Show resolved
Hide resolved
lib/src/main/kotlin/at/bitfire/vcard4android/contactrow/EventHandler.kt
Outdated
Show resolved
Hide resolved
lib/src/main/kotlin/at/bitfire/vcard4android/contactrow/EventHandler.kt
Outdated
Show resolved
Hide resolved
rfc2822
left a comment
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.
Looks good, a few small things
lib/src/main/kotlin/at/bitfire/vcard4android/contactrow/EventHandler.kt
Outdated
Show resolved
Hide resolved
lib/src/main/kotlin/at/bitfire/vcard4android/contactrow/EventHandler.kt
Outdated
Show resolved
Hide resolved
lib/src/main/kotlin/at/bitfire/vcard4android/contactrow/EventHandler.kt
Outdated
Show resolved
Hide resolved
lib/src/main/kotlin/at/bitfire/vcard4android/contactrow/EventHandler.kt
Outdated
Show resolved
Hide resolved
Signed-off-by: Arnau Mora <arnyminerz@proton.me>
Signed-off-by: Arnau Mora <arnyminerz@proton.me>
Signed-off-by: Arnau Mora <arnyminerz@proton.me>
rfc2822
left a comment
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.
Cool. Last minor things, then I'd say its done :)
lib/src/main/kotlin/at/bitfire/vcard4android/contactrow/EventHandler.kt
Outdated
Show resolved
Hide resolved
lib/src/main/kotlin/at/bitfire/vcard4android/contactrow/EventHandler.kt
Outdated
Show resolved
Hide resolved
|
When finished please also request a review from @sunkup |
|
I have updated it & will merge after a copilot review so that I can release 4.5.7-rc.1. |
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.
Pull request overview
This PR adds support for parsing date-time values with timezone information (ZonedDateTime/OffsetDateTime) in the EventHandler class, addressing the need to handle timestamps with time components from Android's AOSP Contacts App. Previously, only date-only formats were supported.
- Refactored date parsing logic to handle both date-time formats (with timezone) and date-only formats
- Added support for parsing partial dates with time components
- Implemented comprehensive test coverage for all supported date/time formats
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| lib/src/main/kotlin/at/bitfire/vcard4android/contactrow/EventHandler.kt | Refactored parsing logic into separate methods (parseFullDate and parsePartialDate) to handle OffsetDateTime, LocalDate, and PartialDate formats; added support for date-time formats with timezone offsets |
| lib/src/test/kotlin/at/bitfire/vcard4android/contactrow/EventHandlerTest.kt | Added comprehensive unit tests for the new parsing methods covering all supported date/time formats including ISO_DATE_AND_TIME_FORMAT, FULL_DATE_FORMAT, and partial date formats with and without time components |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lib/src/main/kotlin/at/bitfire/vcard4android/contactrow/EventHandler.kt
Outdated
Show resolved
Hide resolved
rfc2822
left a comment
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.
I have adapted the last things a bit. Hope everything still works.
See #135 and bitfireAT/davx5-ose#1797.