Allow user to select location of the old content folder during upgrade installation on windows #442

mrpau-richard · 2017-03-07T08:52:58Z

Summary

I created a dialog box to allow user to select the location of ./kalite/content folder if it's not exist during the upgrade.

Issues addressed

Fixes #341

/cc @cpauya @radinamatic

Add input field to set the KA Lite content data path.

mrpau-richard · 2017-03-07T09:00:39Z

Hi @radinamatic can you verify the fix using this windows installer

benjaoming · 2017-04-14T13:41:45Z

Ping @radinamatic :)

benjaoming · 2017-04-27T10:23:34Z

Ping @radinamatic again :)

radinamatic · 2017-04-27T11:47:08Z

On this now!

And btw I'm seeing the CORS header missing error again, while preparing 0.16.9 version for this update scenario:

radinamatic · 2017-04-27T13:01:28Z

Ok, tried the Windows installer linked above, and the implementation of the feature seems to work correctly:

Detects the prior installation and asks if user wishes to upgrade:
Is unable to find the previous .kalite/content folder and allows the user to select it:
Copies the content into the standard location in Users/<username>/.kalite/content/ folder:

However, I'm not sure if this feature only copies the /content/ folder into the new (standard) location, and not the rest of the previous /.kalite/ folder, which would leave the database and locales back. That results in new issues:

User is asked to register the new admin account.
Device needs to be registered again.
Errors on viewing Facilities and Languages:

benjaoming · 2017-04-27T20:58:33Z

@radinamatic that doesn't sound perfect? Copying contents can be very expensive - I thought the point was to either change the CONTENT_ROOT setting or to create a symbolic link (not sure how that works on Windows).

rtibbles · 2017-04-27T20:59:28Z

I think changing the CONTENT_ROOT setting is probably preferable.

radinamatic · 2017-04-28T11:54:25Z

@benjaoming Loosing the previously installed locales is definitely not the end of the world, as contentpacks can be re-downloaded, but I bet nobody would look favorably at loosing the database...

I agree that copying the content may not be the most efficient solution, and if changing the CONTENT_ROOT is better, let's go for it. I'm just saying that if we go the "copying" route, let's copy the database together with the content folder, because that will almost certainly be the expected outcome in case of an upgrade.

benjaoming · 2017-04-28T13:49:40Z

I'm confused now.

Are we talking about allowing the user to select the CONTENT_ROOT (folder with videos) or the user data directory root (also known as ~/.kalite, the one with database and content packs.. and by default also the one with videos) ?

radinamatic · 2017-05-02T11:03:51Z

I am not sure which exact scenario inspired the original issue that prompted the work on this fix, it was long time ago... My guess is that we envisioned the possibility that the previous version stored content in some "non recommended" location, and the decision was to use the user/.kalite/ folder for both videos and databases during the upgrade.

The point is that my original suggestion and this implementation of just copying the content folder is not enough, as it may leave the existing database behind. The solution would be to check for existing database also, and if so, copy it too in the user/.kalite/ folder (we don't have the DATABASE_ROOT, right?). Or don't go the copying route at all, but think of other solution how to make available the old database after the upgrade.

benjaoming · 2017-05-03T12:18:53Z

Closing this because of branch clean-up.

Please reopen for master @mrpau-richard

benjaoming · 2017-05-24T13:19:12Z

@mrpau-richard do you have an answer to @radinamatic 's question above?

However, I'm not sure if this feature only copies the /content/ folder into the new (standard) location, and not the rest of the previous /.kalite/ folder

mrpau-richard · 2017-05-25T06:28:29Z

Hi @radinamatic I agree that setting the CONTENT_ROOT environment variable is more efficient than copying it to ./kalite folder. About the old database I think we can create an option to allow the user to import or not the existing database from the CONTENT_ROOT path.

radinamatic · 2017-05-25T06:40:20Z

@mrpau-richard I agree, let's go with offering the user the option to import the old database during the upgrade! 👍

…-of-the-old-content-folder-during-upgrade-installation-on-windows

…of-the-old-content-folder-during-upgrade-installation-on-windows

mrpau-richard · 2017-05-30T09:10:20Z

@radinamatic here's the windows installer that allow's the user to import an existing database from the selected content folder.

radinamatic · 2017-06-07T14:31:29Z

Ok, so to test this I moved the ./kalite folder outside the user folder, to the root of the C: drive.

The new installer correctly detected the previous installation and reported that no content folder could be found. When prompted I selected the whole ./kalite folder on the root of the C: drive, and the installation continued:

However, once the installation finished and I started the server, it required me to create the admin user again, which was the first sign something has gone awry. So I opened the data.sqlite file, which was 3Kb smaller than original one, and there was no sign of the learner user I previously created:

The original db did have one learner user:

@mrpau-richard Not sure which part has gone wrong, but the original database was not preserved during the upgrade process.

benjaoming · 2017-06-08T14:26:10Z

@radinamatic the database isn't part of the content folder, this feature would only allow using videos stored in a specific location. Can you confirm that this is the intention @mrpau-richard ?

radinamatic · 2017-06-13T10:29:36Z

@mrpau-richard Any reason we are not copying the ./kalite/database folder together with ./kalite/content. I was under the impression that was what we agreed upon few weeks ago...

create an option to allow the user to import or not the existing database.

mrpau-richard · 2017-06-14T08:38:24Z

@radinamatic sorry, I wasn't aware how you test this. When the installer prompted you to select content folder it most be pointed at .kalite/content not in .kalite folder it will also detect existing database to import it. I will also try to fix the conflict after this PR merged.

benjaoming · 2017-06-14T11:23:44Z

@radinamatic @mrpau-richard I think we should only aim for having the content folder imported. This is a far more common use case - we can see about the database folder another time.

radinamatic · 2017-06-15T09:57:26Z

@mrpau-richard My bad, I'll retest and point to .kalite/content folder this time.

@benjaoming
We can, of course, deal with the user database import in another PR, if it's easier to implement separately, but I fail to see why would somebody want only the content and not the all the users and their activity log preserved during the upgrade... 😕

What if the facility had setup several classes/groups with many users? If the database is not imported during the upgrade, coach/admin will have to create them from the scratch, all the data for coach tools will be lost, and all the learners will loose their previously earned points. Am I missing something? Don't we want to preserve that...?

benjaoming · 2017-06-15T14:18:48Z

but I fail to see why would somebody want only the content and not the all the users and their activity log preserved during the upgrade...

You are right about this, but I think it's already preserved?

Actually, I thought the change would also affect fresh installations, in which case I thought it was nice because users may have content sitting on a different drive. For instance, because they are replicating an installation on a new device or have downloaded the contents separately via torrent.

…of-the-old-content-folder-during-upgrade-installation-on-windows

mrpau-richard · 2017-06-16T09:05:24Z

@radinamatic here's the installer to test this PR again.
I also created another issue to allow the user to import their existing database during upgrade.

@benjaoming I agree, That we allow the user to import their 'content' during fresh installation. Are we going to implement it on this PR?

benjaoming · 2017-06-16T13:13:21Z

@benjaoming I agree, That we allow the user to import their 'content' during fresh installation. Are we going to implement it on this PR?

Maybe it's better to say "select" than "import" (as in the title of this PR) because it denotes that files are moved or copied.

If you think it's easy to move this logic so it triggers both on fresh installs and upgrades, that'd be great! 👍

radinamatic · 2017-06-16T13:29:38Z

@benjaoming

You are right about this, but I think it's already preserved?

In update/upgrade scenarios from the more recent versions (where everything is already in user/.kalite/ folder) it is indeed, but I believe here we are exploring the options of what to preserve when the user had the old version of KA Lite in a location which the installer does not recognize, and is not able to preserve the old content and database. The fix here takes care only of the content folder, and I'm trying to "squeeze in" the preservation of the user database, too... 😉

Actually, I thought the change would also affect fresh installations, in which case I thought it was nice because users may have content sitting on a different drive. For instance, because they are replicating an installation on a new device or have downloaded the contents separately via torrent.

All completely true and I support us thinking of these scenarios, I'm just saying we should check if the user database is also sitting somewhere else than the default folder. I see @mrpau-richard already opened another issue to deal with that 👍 👏 , so I'll finish drilling about it here... 😛

…tion.

mrpau-richard · 2017-06-19T07:24:45Z

@radinamatic here's the windows installer that's allow the user to select content folder during fresh and upgrade installation.

radinamatic · 2017-07-05T08:34:53Z

I tested this and it seems to work OK, that is it ask the user to select the .kalite/content/ folder on fresh install.

However, I'm worried if this option results confusing for the new and less savvy users... 🤔
Could we add this phrase at the bottom of the alert window for fresh install?

If this is the first time you are installing KA Lite, or you don't need to import any previous content at this time, press Cancel.

benjaoming · 2017-07-05T09:03:18Z

Suggestion as an alternative to Radina's text:

You can also download contents after the installation. If you don't have a folder with downloaded contents, press Cancel.

The dialogue has the following potential purposes:

Selecting contents from a previous installation
Selecting contents readily downloaded on a non-default location (USB drive etc)
Selecting a content folder on a different drive because the default drive doesn't have enough space (and then downloading contents after the installation)

mrpau-richard · 2017-07-06T08:12:15Z

@radinamatic & @benjaoming I updated the alert message. you can grab the new installer here

windows/installer-source/KaliteSetupScript.iss

+    if Not DirExists(userKaliteContent) then
+    begin
+     if Not DirExists(contentPath) then
+        begin


windows/installer-source/KaliteSetupScript.iss

+        begin
+            if MsgBox('You can also download contents after the installation. If you don`t have a folder with downloaded contents, press Cancel.', mbInformation,  MB_YESNO or MB_DEFBUTTON1) = IDYES then
+            begin 
+               if BrowseForFolder('Please select the .kalite/content/ folder', userPath, False) then


benjaoming · 2017-07-06T10:25:23Z

@mrpau-richard I was commenting in the context of @radinamatic 's comment, so not thinking that my suggestion should stand by itself :)

However, I'm worried if this option results confusing for the new and less savvy users... 🤔
Could we add this phrase at the bottom of the alert window for fresh install?

My suggestion in full would be to replace this text:

'KA Lite Setup is unable to locate the content folder of your previous installation in order to perform the upgrade to current version. Please select the .kalite/content/ folder on your computer and press the button OK to finish the upgrade process.'

With:

'KA Lite Setup cannot find any video contents in the default location (' + contentPath + '). You can download contents after the installation or select a location with previously downloaded contents. If you don't have a folder with downloaded contents, press Cancel. To change the default location of downloaded contents, press OK.'

contentPath should default to userKaliteContent when it's unset.

mrpau-richard · 2017-07-13T08:51:37Z

@benjaoming & @radinamatic I updated the dialog message as you guys suggested. You can grab the installer here for testing.

radinamatic · 2017-07-25T19:07:48Z

I have been re-reading our conversation on this for a while now, have a grim impression that we've lost focus on what we are trying to achieve (probably good part is my fault too 😞), and I think that we need to pause and rewind.

Not sure if it is the formatting issue for the text inside installer dialogs, or there might be some copy/paste errors between various versions of the message here, but I find this wording to be very unclear and potentially confusing to the user, especially if it is their first encounter with KA Lite.

TL;DR version

After spending quite some time on writing all that below, I realized it will be much clearer if I made a drawing... 😛

Yeah, I want to read

If we are to proceed with implementing this complex feature that covers several potential purposes, I would insist we add a conditional first dialog for users who don't need it, or are installing for the first time:

If this is the first time you are installing KA Lite, or you don't need to import any previous local content at this time, skip this step to continue the installation.

Note: You will be able to download videos from the KA Lite management interface.

This dialog should have two buttons at the bottom:

Import content now (first/left)
Skip importing content (second/right; in focus by default, so user just needs to press Enter to continue)

If there is the default content folder from previous installation:

KA Lite Installer found videos in the default location (' + contentPath + ').

Do you want to import this content from the previous installation, or you want to explore locations on other drives?

This dialog should have three buttons at the bottom:

Back (first/left; for users who ended here but changed their mind)
Explore other drives (second/middle)
Import from default location (third/right; in focus by default, so user just needs to press Enter to continue)

If the user chooses Explore other drives installer should open the standard Windows Open folder dialog, and Import from default location should directly proceed to import content. Both options should include a confirmation message at the end, before continuing with the installation.

This should take care of all three proposed purposes:

Selecting contents from a previous installation
Selecting contents readily downloaded on a non-default location (USB drive etc)
Selecting a content folder on a different drive because the default drive doesn't have enough space (and then downloading contents after the installation)

benjaoming · 2017-07-28T13:32:25Z

@radinamatic Just sharing my 2 cents here: Changing the content folder (to another drive for instance) after the installation is quite annoying as it requires:

Editing ~/.kalite/settings.py or doing something with symlinks (which afaik doesn't work on Windows anyways)
Copying and quite possibly merging to different folders (as the original content folder will contain all of the content pack stuff... assessment items, subtitle files etc). You should put all videos (downloaded from for instance torrent) into content/, so you either have to copy existing videos into this folder or the unpacked assessment item stuff into your existing video location.. confusing.

I'm speaking as a user of the Python/Debian stuff... it doesn't matter if you have a helpful wizard or not. Actually, not having it is quite unhelpful, as it will unpack the existing pre-seeded content pack automatically and then you have to stop KA Lite and possibly create a new location for the content folder, symlink stuff (or change settings), copying over all the files of the older content folder..

But I like that we try to ask users the right questions during installation, and I think either way - the current simple dialogue (not sure if the displayed default path is correct) or a more intelligent flow that you suggests - will be a great improvement.

radinamatic · 2017-07-31T15:20:17Z

@benjaoming I misunderstood the last option you were proposing initially - Selecting a content folder on a different drive because the default drive doesn't have enough space (and then downloading contents after the installation) - if I'm understanding it correctly now, it means that user chooses to use another drive as the location for ./kalite/content/ folder, right?

In that case that step (letting the user select the new location where the videos will be) should come before choosing where to search the content to import from, correct? This means that the above workflow needs some adjustments - I'll get right to that!

benjaoming · 2017-07-31T15:48:45Z

In that case that step (letting the user select the new location where the videos will be) should come before choosing where to search the content to import from, correct?

Yes, this is true - thanks for observing this, as I was perhaps a bit in a hurry :) But yes, I'm trying to cater for users that are BOTH 1) installing for the first time and 2) upgrading and 3) re-installing to make another choice for the content folder (as this is a dialogue you do not find in KA Lite itself after installing)

benjaoming · 2017-09-05T07:15:40Z

Note: The goal of closing this issue is to make something that's reusable in Kolibri

radinamatic · 2025-02-11T16:17:28Z

Closing as wontfix

mrpau-richard added 2 commits March 6, 2017 22:54

Create another content page.

26dd327

Add input field to set the KA Lite content data path.

Show dialog box to browse the content folder path.

25c5461

mrpau-richard changed the title ~~Feature/341 allow user to select location of the old content folder during upgrade installation on windows~~ Allow user to select location of the old content folder during upgrade installation on windows Mar 7, 2017

mrpau-richard requested review from cpauya and radinamatic March 7, 2017 09:02

mrpau-richard closed this Apr 27, 2017

mrpau-richard reopened this Apr 27, 2017

benjaoming closed this May 3, 2017

mrpau-richard changed the base branch from develop to master May 4, 2017 02:05

mrpau-richard reopened this May 4, 2017

mrpau-richard added 3 commits May 26, 2017 15:20

Merge branch 'develop' into feature/341-allow-user-to-select-location…

8c0cffb

…-of-the-old-content-folder-during-upgrade-installation-on-windows

Merge branch 'master' into feature/341-allow-user-to-select-location-…

fbdd24d

…of-the-old-content-folder-during-upgrade-installation-on-windows

Create an option to import database from content folder.

a992adf

Merge branch 'master' into feature/341-allow-user-to-select-location-…

c09eaff

…of-the-old-content-folder-during-upgrade-installation-on-windows

mrpau-richard mentioned this pull request Jun 16, 2017

Allow user to import KA Lite database during upgrade process #459

Open

Remove allowing the user to import there existing KA Lite database.

8aabe21

Allow user to select content folder during fresh and upgrade installa…

29bd491

…tion.

benjaoming changed the base branch from master to 0.17.x June 28, 2017 12:50

Update alert message when asking user to import contents.

7c6f57b

benjaoming suggested changes Jul 6, 2017

View reviewed changes

Remove if conditions in checking content folder.

22b1063

mrpau-richard mentioned this pull request Sep 1, 2017

Allow user to import KA Lite database during upgrade. #462

Closed

radinamatic closed this Feb 11, 2025

Allow user to select location of the old content folder during upgrade installation on windows #442

Allow user to select location of the old content folder during upgrade installation on windows #442

Conversation

mrpau-richard commented Mar 7, 2017 • edited Loading

Summary

Issues addressed

mrpau-richard commented Mar 7, 2017

benjaoming commented Apr 14, 2017

benjaoming commented Apr 27, 2017

radinamatic commented Apr 27, 2017

radinamatic commented Apr 27, 2017

benjaoming commented Apr 27, 2017

rtibbles commented Apr 27, 2017

radinamatic commented Apr 28, 2017 • edited Loading

benjaoming commented Apr 28, 2017

radinamatic commented May 2, 2017

benjaoming commented May 3, 2017

benjaoming commented May 24, 2017

mrpau-richard commented May 25, 2017

radinamatic commented May 25, 2017

mrpau-richard commented May 30, 2017

radinamatic commented Jun 7, 2017

benjaoming commented Jun 8, 2017

radinamatic commented Jun 13, 2017

mrpau-richard commented Jun 14, 2017

benjaoming commented Jun 14, 2017

radinamatic commented Jun 15, 2017

benjaoming commented Jun 15, 2017

mrpau-richard commented Jun 16, 2017 • edited Loading

benjaoming commented Jun 16, 2017

radinamatic commented Jun 16, 2017

mrpau-richard commented Jun 19, 2017

radinamatic commented Jul 5, 2017

benjaoming commented Jul 5, 2017 • edited Loading

mrpau-richard commented Jul 6, 2017 • edited Loading

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

benjaoming commented Jul 6, 2017

mrpau-richard commented Jul 13, 2017

radinamatic commented Jul 25, 2017

TL;DR version

Yeah, I want to read

benjaoming commented Jul 28, 2017 • edited Loading

radinamatic commented Jul 31, 2017 • edited Loading

benjaoming commented Jul 31, 2017

benjaoming commented Sep 5, 2017

radinamatic commented Feb 11, 2025

mrpau-richard commented Mar 7, 2017 •

edited

Loading

radinamatic commented Apr 28, 2017 •

edited

Loading

mrpau-richard commented Jun 16, 2017 •

edited

Loading

benjaoming commented Jul 5, 2017 •

edited

Loading

mrpau-richard commented Jul 6, 2017 •

edited

Loading

benjaoming commented Jul 28, 2017 •

edited

Loading

radinamatic commented Jul 31, 2017 •

edited

Loading