Feat : Add download update feature in user app. Issue #755

[shashank40]

/claim #755

What kind of change does this PR introduce?

Feature addition

Summary

Currently we don't support app update functionality in our app. With this addition, user can get the latest version of app from the app-tray itself.

1. App shows latest version to download if available
2. Else it shows no update found.

Video

screen-recording-2024-06-19-at-43728-pm_NVMCrxmk.mp4

Checklist

• My code follows the style guidelines of OpenAdapt
• I have performed a self-review of my code
• If applicable, I have added tests to prove my fix is functional/effective
• I have linted my code locally prior to submission
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation (e.g. README.md, requirements.txt)
• New and existing unit tests pass locally with my changes

How can your code be run and tested?

Other information

[shashank40]

Hi @abrichr
I have implemented the feature to allow the user to download the latest app when needed.

Now why should we do this and not automatic update? As this is an Industry standard.

No app downloads the updates on every start as :

1. This makes the app slow
2. We are forcing an update on the user.
3. User needs to have multiple command line tools to build it on the go. This is not at all ideal.

So this comment was the best solution and i have implemented that.

I have also added a video in the description showing how the feature works.

[abrichr]

@shashank40 this a great start!

In the comment you linked, the third step is:

On clicking that, the newest version should be downloaded, and unzipped in the same directory as where the current app lives. There are no additional installation instructions for OpenAdapt, so we could just display a message box saying that the new app was downloaded, and maybe delete the current version as well (@abrichr thoughts on this as well)

If you implement this we can consider this. Keep going!

[shashank40]

@abrichr
Just a thought!
According to me unzipping in the download directory better than current directory as people generally look for downloads in the downloads folder rather than any random directory)? (Your thoughts @abrichr?)

I think that is the only part left in the comment.

But i will implement the unzipping part for sure.

[shashank40]

Also @abrichr
i dont think we should implement deleting the old version.
Reason : what if the user is in a middle of the recording during download? And we delete this old build, this is not ideal

I think extracting the zip is the best thing we should do and not the deleting part.

Thoughts? @abrichr

[abrichr]

@shashank40 typically how applications handle this is to say something like:

A new version is available: vxx.xx.xx (you are on vyy.yy.yy). Would you like to update? Yes/No/Skip this version

We can show a message on startup indicating that a new version is available, and let the user decide how to proceed.

[shashank40]

@abrichr
Yess i agree, but isn't this similar to what is implemented but better?. Rather than popup we have it in a tray with the version to update visible.
User can choose to implement and not to implement.

Why i say this is, as OpenAdapt gets latest updates quite frequently(sometimes even 2-3 a day) and not weekly/monthly. So let's say i update it once and then few hours later there would be another popup hindering me.

This would be a bad user experience every time i open an app.

The implementation that you suggested might work better for apps/softwares that have scheduled updates that come weekly/monthly. But with these frequent updates, our user experience will go down.

[abrichr]

It's an interesting point @shashank40. But I still think we want to make the user's life easier by automatically extracting the application for them if they do choose to update.

[shashank40]

@abrichr You mean extracting the zip file?

[shashank40]

@abrichr Done.
Added the functionality to unzip as well

[abrichr]

@shashank40 I'm unable to test this on my machine because of #785.

Can you please record a video showing the updated functionality?

[abrichr]

@shashank40 if you can upload a video showing this working, address my comments, and fix the formatting issues causing the build to fail, then I think we can get this merged!

[shashank40]

Surely @abrichr , will update this in sometime. Currently AFK.

[shashank40]

Hi @abrichr.

I have replied back to the comments. I am also adding a new video here.

In the video you can see the latest version now shows as 0.34.0 and in yesterday's video it was 0.33.2 and so this makes the app quite reactive to new updates. That will be the beauty of this new update. Dynamically telling you the latest version available.

This is a full video of download process so from start till 3:30 mins it mostly shows download > then it shows unzipping > then it shows the unzipped new version available for the user.

screen-recording-2024-06-20-at-103012-pm_aXnlm7SD.mp4

[shashank40]

Hey @abrichr can we go ahead and merge this?

[shashank40]

@abrichr any update on this?

[abrichr]

Hi @shashank40 , this looks good. However it's unfortunate that there is no visual indicator for the user about the progress, other than what is in the terminal (users who run the app not from the terminal won't see any progress).

I think we need some sort of status indicator, e.g. via show_toast. What do you think?

[abrichr]

Suggested change

	import requests
	import threading
	from tqdm import tqdm
	import time
	from tqdm import tqdm

[shashank40]

Did this with isort

[abrichr]

Thank you @shashank40 ! Almost there.

[abrichr]

I would expect this message to be symmetrical to the success case, e.g.:

"Download & Unzipping Complete"
...
"Download & Unzipping Failed"

Perhaps instead of Download & Unzipping it can be simply Update

Also, should we rename download_complete to e.g. update_complete?

[shashank40]

@abrichr In my opinion download is a better word. We haven't updated the app. We are downloading the latest update of the app. Updating would mean updating the current app they are using, which is not true.

[abrichr]

Please use logger instead of print

[shashank40]

Done

[abrichr]

What do you think about adding the suffix _signal to each of these?

[shashank40]

Done

[shashank40]

Hi @abrichr
Made all the specified changes. I hope its better.

[shashank40]

@abrichr any update?

[abrichr]

@shashank40 can you please clarify why -50?

If this is due to some sort of race condition, what do you think about destroying the existing toast before displaying a new one? e.g. with self.download_progress_toast.hide()

[shashank40]

Yess @abrichr , it was to handle condition between spitting new toast and old toast times out by then.
But i have changed it to .hide()

[shashank40]

@abrichr Can you please do a final review?

[abrichr]

Thank you @shashank40. There appear to be a few loose ends:

1. The "extracting" toast disappears, and there is no indication to the user that anything is happening. Ideally we show progress on this as well, e.g. using the zip_file module:

def extract_with_progress_zip(zip_path: str, extract_to: str):
    with zipfile.ZipFile(zip_path, 'r') as zip_ref:
        total_files = len(zip_ref.infolist())
        for i, file in enumerate(zip_ref.infolist()):
            zip_ref.extract(file, extract_to)
            logger.info(f"Extracted {i + 1} of {total_files} files ({(i + 1) / total_files * 100:.2f}%)")
            # TODO: update toast, like with downloading

2. Once unzipping is complete, the user will have more than one copy of OpenAdapt on their hard drive. Ideally we:
   a. Move the newly extracted version to the same directory as the old version.
   b. Ask the user whether they would like to remove the previous version, and if so then remove it*.

3. Finally, I would expect that:
   a. the currently running version of OpenAdapt is shut down
   b. the newly extracted version's executable is executed

• The removing will probably need to be done on the first launch of the new version.

I would like to merge this now, but as it currently stands, it's not very user friendly without these additions, i.e. once the extraction is complete, the user likely expects to be running the new version, even though currently they won't be.

Once these additions have been implemented, the update feature will be complete and we can merge this.

Thank you @shashank40 , almost there!

Edit: implementation, courtesy of ChatGPT:

---

import os
import sys
import shutil
import subprocess
import atexit
from PySide6.QtWidgets import QMessageBox, QApplication

def get_current_executable_path() -> str:
    if hasattr(sys, '_MEIPASS'):
        # If running from a PyInstaller bundle
        return sys.executable
    else:
        # If running as a script
        return os.path.abspath(__file__)

def get_target_path() -> str:
    current_executable_path = get_current_executable_path()
    return os.path.dirname(current_executable_path)

def move_new_version(extracted_path: str, target_path: str) -> bool:
    try:
        # Move contents of the new version to the target path
        for item in os.listdir(extracted_path):
            s = os.path.join(extracted_path, item)
            d = os.path.join(target_path, item)
            if os.path.isdir(s):
                shutil.copytree(s, d, dirs_exist_ok=True)
            else:
                shutil.copy2(s, d)
        return True
    except Exception as e:
        print(f"Error moving the new version: {e}")
        return False

def ask_user_to_remove_old_version() -> bool:
    app = QApplication([])
    msg_box = QMessageBox()
    msg_box.setText("Would you like to remove the previous version of OpenAdapt?")
    msg_box.setStandardButtons(QMessageBox.Yes | QMessageBox.No)
    reply = msg_box.exec()

    if reply == QMessageBox.Yes:
        return True
    else:
        return False

def remove_old_version(old_version_path: str):
    try:
        shutil.rmtree(old_version_path)
        print("Old version removed successfully.")
    except Exception as e:
        print(f"Error removing old version: {e}")

def shutdown_current_version():
    QApplication.quit()
    sys.exit(0)

def launch_new_version(new_executable_path: str):
    def launch():
        try:
            subprocess.Popen([new_executable_path])
        except Exception as e:
            print(f"Error launching the new version: {e}")

    atexit.register(launch)

def main():
    # Paths for the extracted new version and target location
    extracted_path = "/path/to/extracted/new/version"
    target_path = get_target_path()
    new_executable_path = os.path.join(target_path, "openadapt_executable")

    # Move the new version to the target location
    if move_new_version(extracted_path, target_path):
        # Ask the user if they want to remove the old version
        if ask_user_to_remove_old_version():
            # On the first launch of the new version, remove the old version
            remove_old_version(target_path)

        # Schedule the new version to launch after shutdown
        launch_new_version(new_executable_path)

        # Shut down the current version
        shutdown_current_version()

if __name__ == "__main__":
    main()

[shashank40]

@abrichr I think we should not do part 1), reason is extraction takes 5-10secs. Now we already show toast when it starts and when extraction completes. The overhead and resource consumption to show extraction time left(which in turn is shown by 2 different toast) seems excessive.

For 2, again i am less aligned. The things we are trying to implement happens in installed apps and not desktop apps which OpenAdapt is. I think we should compare OpenAdapt to Desktop Apps and not AppStore applications. In desktop apps, if we download 10 different ZIP files which have 10 different versions , they all stay and is manually deleted by the user if needed. A zip/unzipped file is never deleted by an app as this is being intrusive. It is always the decision of a user to delete the the file he downloaded manually.
There are many reasons for it :

1. This will have strict permission requests from OS and can mark the app dangerous, specially for mac because if the app gets the permission to delete one file, it can be directed to delete any number of files from a personal computer, which is dangerous.
2. I have never seen desktop apps deleting manually downloaded files and we should stick to the norm. If a user is downloading something, we should let the user decide what he wants to do with that.
3. If the user selects yes on delete the old version and the new version is faulty, he won't be able to download the old version again, which again ruins the user experience.
4. Also deleting a file automatically is not at all enhancing the user experience as compared to the intrusion its causing.

Eventually its your take, but i would never install an app that can delete files from my personal computer. I would rather choose to delete a file manually(as it literally takes 1/2 a sec) than installing an app that can delete files.

[abrichr]

Thank you @shashank40 , I appreciate you presenting an alternative perspective.

Regarding extracting the zip file: on my machine, it can take a while. The notification disappears before the extraction is complete, then it's not clear whether the process is complete or not. I think it's important we implement similar logic as for downloading to indicate the update is still occurring.

Regarding not removing the old application, the intention here is to emulate the behavior of other applications that update automatically. For example, when I update Chrome, I don't have to remove the old directory. What do you think?

[shashank40]

@abrichr When comparing it to chrome, browsers are installed applications which have their own directory on the hard-drive and change required files when updated. So they never touch any other directory/downloaded file. Even if we download a multiple zips for chrome, they are never deleted by chrome.

So installed apps work very differently in how updates work and how files are changed in their installed directory locations.

OpenAdapt is a desktop app and not and installed app. It doesn't have a fixed directory like installed apps and so deleting files from any other directory needs higher level permissions and makes it unsafe. If OpenAdapt can delete files from any directory, it makes it unsafe.

[shashank40]

@abrichr updates?

[abrichr]

Hi @shashank40 , thank you for your patience.

OpenAdapt is a desktop app and not and installed app.

We've implemented installers in #858. We would like the update functionality to work with the installed app.

Edit: I know you're been at this one for a while. Almost there!