-
Notifications
You must be signed in to change notification settings - Fork 52
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
Handle data migration for the tables renamed after v2.2.0 #340
base: master
Are you sure you want to change the base?
Handle data migration for the tables renamed after v2.2.0 #340
Conversation
hello @floss4good thanks for your contribution. Yeah it was for future, but android permissions and google play store removed us then i stopped the project after battling for months with google play bots to try to re-enable the app... |
it may also be harder to me to test it as now i'm using signal mainly on my phone and not the original SMS backed backend |
Confirm working #329 (comment) THANKS ! |
As I've had previous data I didn't want to lose, and NextCloud
Doing that and fixing CSS as indicated in #347 (comment) made ocsms work for me again (until next time...) |
@mnalis, since Nextcloud version 22 the Anyway, I should probably make some time and rework this PR and hopefully someone could review it. |
@floss4good do you perhaps have it working in NextCloud 29 (#356)? |
@mnalis I'm still on an older Nextcloud version (mainly because I still use ocsms) and I've been postponing the work on this for ~2 months already because of other priorities; so I can't promise anything right now. |
This migration will copy data from
ocsms_conversation_read_states
andocsms_sendmessage_queue
legacy tables toocsms_conv_r_states
andocsms_sendmess_queue
, respectively, new tables after which will perform the following DB schema changes:ocsms_conversation_read_states
legacy table (if data successfully copied to new table);ocsms_sendmessage_queue
legacy table (if found);ocsms_smsdatas
table:smsdata_user_mailbox
→smsdata_user_mbox
smsdata_user_mailbox_date
→smsdata_user_mbox_date
smsdata_user_mailbox_address
→smsdata_user_mbox_addr
smsdata_user_mailbox_address_date
→smsdata_user_mbox_addr_date
With this it will be possible to update the DB for already installed versions.
I only tested it with Nextcloud 20.0.12 by manually running the
occ migrations:migrate ocsms
command and also with a fresh install of the Phone Sync application. Not tested with Nextcloud 21 or newer since the app is currently configured to work between versions 18 and 20.Since this is my first contribution to the Nextcloud ecosystem – therefore also my first for this project – please find below some remarks:
I named my migration class/file Version020300Date20210926000100 assuming that the next version to be released will be 2.3 however, it is not clear to me what version should be used (the one from which you are migrating or the one to which you are migrating). If someone can clarify this aspect, I can just rename the class/file, if needed.
I noticed that migration Version020109Date20201216203338 was altered although it was already included in version 2.2.0 (see commit 224382c) in order to rename the tables and indexes – effective in case of new installations – that are also subject of current migration. From my understanding this is a bad practice and these changes should have been done in a new migration. Check the second Note from documentation:
If you want I can revert the changes and adjust my pull request accordingly.
One of the renamed tables – ocsms_sendmessage_queue – seems to not be used at all. From my point of view, if someone (@nerzhul, most probably) can confirm this, it would be a good moment to drop this table and remove the useless data migration part from current migration file. However, I will open a distinct issue for this.