Cht-core: Horticulturalist shouldn't show older versions

Created on 25 May 2018  路  12Comments  路  Source: medic/cht-core

Since we don't support upgrading to an older version, we should not show the versions in horti. The other option (If we want to show the list of versions) could be to gray out the upgrade buttons for the older versions.

2 - Medium Improvement

Most helpful comment

OK so I am definitely not in touch with the kids on this one, reopening :P

In terms of the ideas above, the hard part to me is:

  • Working out what older means, usefully (Gareth's suggestion about a base package.json version is I think pretty good, but I wouldn't want to block anyone from doing anything based on that, and it still is only a nod toward compatibility, it doesn't prove anything specifically).
  • Working out how to show that in a way that keeps the page simple, so it doesn't turn the a legal document of warnings, notes and differing actions. No one reads warnings anyway, and the more you add the even less they read them. We need to allow for people to deploy arbitrary things, so we can't disable it completely, which then necessitates some kind of visual queues or something.

@alxndrsn / @garethbowen / @henokgetachew how do you feel about the following:

  • adding a base_version to build_info that has whatever version we have in the package.json at the time.
  • When deployed as a branch, display the base version along with the branch name at the top of the deployment page
  • When displaying branches to deploy to, also display their base version somehow (perhaps like some_branch_name (~3.0.0))
  • Somehow indicate near the stage / deploy buttons when the base version is lower than your current base version. I would vote for a 鈿狅笍 to the left with hover text informing the user that this version is an older base version than they currently have, and may cause problems
  • Not actually stop anyone from deploying from master -> 2.14.0 or random_branch to random_second_branch or whatever, if they chose to do that

All 12 comments

We already do what you want, in production situations.

Specifically (for now, we could change how we detect this), if you're deployed as *.app.medicmobile.org you can only take final release versions, and only versions newer than what you're on.

If you run it in any other way you can do whatever you like. In part because this is not production so we can be a bit more cavalier with what we allow (e.g. maybe you want to move between beta versions to see if something broke between them, and you know there are no migrations or otherwise breaking changes between them and so it's fine).

Much more importantly though, non-production deployments allow for deploying of branches, and unless we start analysing the git tree we don't know what versions are and aren't valid, and even then once you'd deployed a single branch you wouldn't be able to deploy anything else.

So I'm going to close this. I noticed that the discussion in slack also contained @garethbowen and @alxndrsn, so I'm tagging you in case you disagree with what I'm saying and want to re-open.

@SCdF the production situation sounds fine. For non-prod: if currently it's unclear to users, I'd suggest doing something in the UI to make it more obvious that a particular version might be a downgrade. Some things that might help:

  • greying the names of older versions
  • adding a confirmation dialog when an older version is selected
  • showing a tooltip on-hover saying something like _this version may be older than your current version_
    In instances where we can't know, perhaps making _that_ clear would help. E.g. _This build is from a different branch: there may not be a supported upgrade path when switching._

I agree I think we can do better for non-prod deployments. For example, if we store the package.json version with the artefact as well as the branch name we can analyse it to allow switching between branches with the same root version.

OK so I am definitely not in touch with the kids on this one, reopening :P

In terms of the ideas above, the hard part to me is:

  • Working out what older means, usefully (Gareth's suggestion about a base package.json version is I think pretty good, but I wouldn't want to block anyone from doing anything based on that, and it still is only a nod toward compatibility, it doesn't prove anything specifically).
  • Working out how to show that in a way that keeps the page simple, so it doesn't turn the a legal document of warnings, notes and differing actions. No one reads warnings anyway, and the more you add the even less they read them. We need to allow for people to deploy arbitrary things, so we can't disable it completely, which then necessitates some kind of visual queues or something.

@alxndrsn / @garethbowen / @henokgetachew how do you feel about the following:

  • adding a base_version to build_info that has whatever version we have in the package.json at the time.
  • When deployed as a branch, display the base version along with the branch name at the top of the deployment page
  • When displaying branches to deploy to, also display their base version somehow (perhaps like some_branch_name (~3.0.0))
  • Somehow indicate near the stage / deploy buttons when the base version is lower than your current base version. I would vote for a 鈿狅笍 to the left with hover text informing the user that this version is an older base version than they currently have, and may cause problems
  • Not actually stop anyone from deploying from master -> 2.14.0 or random_branch to random_second_branch or whatever, if they chose to do that

Sounds good to me. It might be nice to hide the old branches by default (accordion or something?) because there are quite a few of them now. This can be done later.

It might be nice to hide the old branches by default (accordion or something?)

Yeah this is a good idea in general. Right now we just show everyone everything and it's not that readable.

@rmhowe plz review. If you merge it no need to squash, each commit makes sense on its own!

Done.

We now show a warning sign against branches you may not be compatible with. This is based on a "base version", which is the root version your branch is based on. Since this requires a new piece of information, all current branches have this warning. Over time this will go away as people branch off the new master with this information.

We already do what you want, in production situations.

Do we have any production instances running with horti? Otherwise, it looks good to have the ability to go back in time in test and dev environments. Maybe we might need an easy recovery mechanism or a fail-safe method, just in case someone inadvertently upgrades to an incompatible version and breaks the app( especially since the warning sign is not bold enough for someone who is not aware of this conversation). But again, it is not super important in the grander scheme of things, since it is not production. So, I will leave this open for @henokgetachew to confirm on production deployments.

Do we have any production instances running with horti?

Not yet unfortunately!

Maybe we might need an easy recovery mechanism or a fail-safe method,

Apart from backing up the entire server I don't think there is anything we could do.

Think of all the tools you use. Your computer lets you delete files, git let's you force push, rm lets you rf.

My philosophy here is that tools exist to enable you to do things. Warnings are definitely useful, but it's really hard to create them in a such a way where they won't just be ignored, and counter-intuitively research has shown making them large, bolder and more obstructive doesn't help, it makes people ignore them more.

@SCdF @ngaruko this looks to have added a check for version incompatibility and a warning icon on the upgrade instances page for potentially incompatible versions right? This is showing on my local env for previous branches and since we don't have any running in production currently is there a reason to keep this open?

@newtewt I'm happy with your assessment there

Was this page helpful?
0 / 5 - 0 ratings