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

Enabled automatic id generation #117

Merged
merged 4 commits into from
Feb 21, 2023

Conversation

ArnyminerZ
Copy link
Member

Closes #116

rfc2822 and others added 2 commits February 21, 2023 12:04
Signed-off-by: Arnau Mora <arnyminer.z@gmail.com>
@ArnyminerZ ArnyminerZ added the bug Something isn't working label Feb 21, 2023
@ArnyminerZ ArnyminerZ linked an issue Feb 21, 2023 that may be closed by this pull request
@ArnyminerZ ArnyminerZ marked this pull request as ready for review February 21, 2023 11:18
@rfc2822
Copy link
Member

rfc2822 commented Feb 21, 2023

Please rebase/merge again, I updated dependencies in the dev branch

…endar-when-adding-a-subscription

# Conflicts:
#	app/build.gradle
Copy link
Member

@rfc2822 rfc2822 left a comment

Choose a reason for hiding this comment

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

Does this change need a migration and/or migrate automatically?

@ArnyminerZ
Copy link
Member Author

@rfc2822 It should work out without any issues. It's just for new subscriptions. The old ones directly retain the ids provided by the Android calendar. See here:

/**
* Converts a [LocalCalendar] to a [Subscription] data object.
* Must only be used for migrating legacy calendars.
*
* @param calendar The legacy calendar to create the subscription from.
* @return A new [Subscription] that has the contents of [calendar].
*/
fun fromLegacyCalendar(calendar: LocalCalendar) =
Subscription(
id = calendar.id,
url = Uri.parse(calendar.url ?: "https://invalid-url"),
eTag = calendar.eTag,
displayName = calendar.displayName ?: calendar.id.toString(),
lastModified = calendar.lastModified,
lastSync = calendar.lastSync,
errorMessage = calendar.errorMessage,
ignoreEmbeddedAlerts = calendar.ignoreEmbeddedAlerts ?: false,
defaultAlarmMinutes = calendar.defaultAlarmMinutes,
color = calendar.color
)

@rfc2822
Copy link
Member

rfc2822 commented Feb 21, 2023

I meant whether this adds an AUTOINCREMENT to the SQLite database or something like that. Then we would get a new database schema and would have to add an (auto-)migration.

@ArnyminerZ
Copy link
Member Author

ArnyminerZ commented Feb 21, 2023

I meant whether this adds an AUTOINCREMENT to the SQLite database or something like that

Mmmh, I'm not sure, I'll look into it 😉

Edit: Yup, that's exactly what it does. It adds PRIMARY KEY AUTOINCREMENT

@ArnyminerZ
Copy link
Member Author

ArnyminerZ commented Feb 21, 2023

@rfc2822 Is the version already released? I mean, if only us have it, maybe we can just erase data and not have to add a new version and enable automatic migration

@rfc2822
Copy link
Member

rfc2822 commented Feb 21, 2023

True, it's not released yet, so this would work (we should do a manual extra test whether the release upgrade from the latest released version to now works smooth).

But shouldn't the change also update https://github.com/bitfireAT/icsx5/blob/dev/app/schemas/at.bitfire.icsdroid.db.AppDatabase/1.json?

@ArnyminerZ
Copy link
Member Author

But shouldn't the change also update https://github.com/bitfireAT/icsx5/blob/dev/app/schemas/at.bitfire.icsdroid.db.AppDatabase/1.json

Yes, I was waiting for your confirmation 😄

Signed-off-by: Arnau Mora <arnyminer.z@gmail.com>
Copy link
Member

@rfc2822 rfc2822 left a comment

Choose a reason for hiding this comment

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

Seems to work now!

@rfc2822 rfc2822 merged commit 404bb9b into dev Feb 21, 2023
@rfc2822 rfc2822 deleted the 116-couldnt-create-calendar-when-adding-a-subscription branch February 21, 2023 22:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Couldn't create calendar" when adding a subscription
2 participants