Skip to content
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

Fix calendar handling of pre-1970 birthdates #473

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

LFd3v
Copy link

@LFd3v LFd3v commented Feb 6, 2025

Re submit PR #224, all credits to @TristanDonze.

I cannot do a build right now, but I can more than happily test a beta build if needed (and update the PR with any necessary changes if required, of course).

What is it?

  • Bugfix
  • Feature
  • Codebase improvement

Description of the changes in your PR

  • Removed lines of code.

Before/After Screenshots/Screen Record

Fixes the following issue(s)

Acknowledgement

Re submit PR FossifyOrg#224, all credits to @TristanDonze.

I cannot do a build right now, but I can more than happily test a beta build if needed.
@naveensingh naveensingh added the testers needed We need testers for this issue or pull request label Feb 6, 2025
@naveensingh
Copy link
Member

I cannot do a build right now, but I can more than happily test a beta build if needed.

A test build should be available under checks tab soon :)

@LFd3v
Copy link
Author

LFd3v commented Feb 6, 2025

A test build should be available under checks tab soon :)

Awesome. I did not see any potential side effects, but running a test build is always a good idea. Thanks and sorry for the trouble.

@LFd3v
Copy link
Author

LFd3v commented Feb 6, 2025

Hmm, I think now I understand why the check for years before 1970 was added: for events (birthdays or anniversaries) without an year, the age is calculated using the year 1604, resulting in ages like 420 years for these events in 2024, for instance. Anyway, limiting the date to 1970 is even worse IMHO...

  1. Probably the best solution would be not printing the age for events without an year, of course. But then it will require more changes and tests.

  2. A simpler solution would be checking for the full year, and set it to the current year so the age would be zero. I can try to implement this.

@naveensingh Thoughts? Do you want me to simply add a TODO to remind about item 1) or try to do 2)?

@naveensingh
Copy link
Member

naveensingh commented Feb 6, 2025

Probably the best solution would be not printing the age for events without an year, of course. But then it will require more changes and tests.

This is the way. I don't think we need a lot of changes for it, just adding FLAG_MISSING_YEAR flag to the birthday event should do the job as this is already implemented for public contacts (see addContactEvents()).

var missingYear = false
// private contacts are created in Simple Contacts Pro, so we can guarantee that they exist only in these 2 formats
val format = if (birthdayAnniversary.startsWith("--")) {
     missingYear = true
     "--MM-dd"
} else {
    "yyyy-MM-dd"
}

var flags = if (missingYear) {
    FLAG_ALL_DAY or FLAG_MISSING_YEAR
} else {
    FLAG_ALL_DAY
}

// use flags...

I have not tested this but it should work.

Events without an year are not handled properly, this should fix it as discussed [here](FossifyOrg#473 (comment)).
@LFd3v
Copy link
Author

LFd3v commented Feb 6, 2025

Oh, great. I missed the block above for other events, it indeed seems to handle events without an year properly.

I was going to ask to to trigger a new build in my fork, but it seems that it was triggered here already. I hope it works fine now!

@LFd3v
Copy link
Author

LFd3v commented Feb 6, 2025

It seems to be working just fine:

  • events before 1970 now display the correct age
  • events with hidden year now have the age hidden in the app
  • no crashes or other visible glitches

Just a remark: in the release notes, it may be a good idea to point out that birthdays and anniversaries will have to be imported again so the Calendar app will display them properly.

Cheers!

@naveensingh
Copy link
Member

Great, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testers needed We need testers for this issue or pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Age incorrect for private contacts from Fossify Contacts who are born before 1970
2 participants