multi: show upgrade banner if newer version is available on GitHub #3286

ukane-philemon · 2025-05-22T23:23:33Z

https://github.com/user-attachments/assets/7a077e62-6a2b-4744-99d2-d3e9beb6532a

https://github.com/user-attachments/assets/ecdaa6fc-99e0-436f-91ad-a11756af3eb7

The banner stays until the user upgrades.

PS: The wording was changed to direct users to the bison wallet; this is just a visual representation of the banner.

Closes #3191

Signed-off-by: Philemon Ukane <[email protected]>

client/webserver/webserver.go

JoeGruffins · 2025-05-23T06:18:16Z

client/webserver/webserver.go

+				continue
+			}
+
+			w.latestVersion = strings.Trim(response.TagName, "v")


It's possible this isnt just v1.x.x https://api.github.com/repos/decred/dcrd/releases/latest

Possible yes but we’ll have to work with how dcrdex name releases, not sure why that should change.

Errm, prior to v0.6.x it was name differently "DCRDEX v0.x.x" but going forward I think we should stick to one way of doing things, using the vx.x.x as tag name.

568af47 introduced gt check that should resolve your concerns @JoeGruffins.

Should proly use a regex here to future-proof this.

Review comment: #3286 (comment)

Signed-off-by: Philemon Ukane <[email protected]>

exitus1 · 2025-05-23T16:49:21Z

Great feature, though wouldn't it make more sense to direct people to the website?

dev-warrior777 · 2025-05-23T16:53:05Z

Great feature, though wouldn't it make more sense to direct people to the website?

It would if the website and github were tightly in sync. Often there is a lag in updating the website.

exitus1 · 2025-05-23T16:58:08Z

Great feature, though wouldn't it make more sense to direct people to the website?

It would if the website and github were tightly in sync. Often there is a lag in updating the website.

This is true...but I'm just sayin' the /average user/ isn't exactly used to downloading things from Github where you have 16 different files to select from. Is updating the website when there is a new release a difficult task?

ukane-philemon · 2025-05-23T17:16:08Z

Great feature, though wouldn't it make more sense to direct people to the website?

It would if the website and github were tightly in sync. Often there is a lag in updating the website.

This is true...but I'm just sayin' the /average user/ isn't exactly used to downloading things from Github where you have 16 different files to select from. Is updating the website when there is a new release a difficult task?

I thought about all of the above, and now that it's also mentioned..I'll just redirect them to the website.

Is updating the website when there is a new release a difficult task

I suppose there should be a release checklist, right? @buck54321

Signed-off-by: Philemon Ukane <[email protected]>

client/webserver/webserver.go

buck54321 · 2025-05-24T12:42:17Z

The banner is too aggro. Just use a notification.

ukane-philemon · 2025-05-24T12:58:56Z

IMO an upgrade is a highly recommended action that is not part of the actual function of the app...it could easily be ignored if sent via notes.

client/core/core.go

JoeGruffins · 2025-05-26T06:21:29Z

client/core/core.go

+// GitHub API for the latest release. If a new release is found, it notifies the
+// user with a TopicNewReleaseAvailable note.
+func (c *Core) checkForNewRelease(ctx context.Context) {
+	ticker := time.NewTicker(3 * time.Minute)


Sorry, I didn't notice there were two for loops selecting of a case. One minute or so is fine here I guess. Would be good to try once without waiting as well.

If we try once, and it always fails, the user may theoretically never get the note, (i.e if they never run the app for 24hrs)

JoeGruffins · 2025-05-26T06:26:21Z

client/core/core.go

+	currentVersion := strings.Split(c.cfg.Version, "-")[0]
+	if latestVersion > currentVersion && !strings.Contains(latestVersion, currentVersion) {


latestVersion > currentVersion isn't good for string comparison. consider that "v1.0.2" > "1.0.3" will return true. I assume there's a way to do better by splitting on decimals https://stackoverflow.com/questions/8955657/regex-pattern-to-extract-version-number-from-string

I think tests are passing because of the !strings.Contains part

Sorry, use this

dcrdex/dex/version/version.go

Line 48 in 2b78ab8

func ParseSemVer(s string) (major, minor, patch uint32, preRel, build string, err error) {

Sorry, use this

dcrdex/dex/version/version.go

Line 48 in 2b78ab8

func ParseSemVer(s string) (major, minor, patch uint32, preRel, build string, err error) {

This would work for c.cfg.Version, but we'd proly need the regex suggestion for latestVersion.

Could reuse the userAppVersion function in #3289, might move the helper function to dex/version.

JoeGruffins

I'm not seeing the notification in the ui for some reason. I see it in logs though.

ukane-philemon · 2025-05-26T11:34:30Z

I'm not seeing the notification in the ui for some reason. I see it in logs though.

Check the Recent Activity in the Notification dropdown, poke level is < success level so it won't show up in the Notification section.

martonp

The github check looks good. I think for the UI, instead of having it in the notifications, there should be a bar on the top of the screen just sitting there all the time telling the user to update to a new version.

ukane-philemon · 2025-05-28T20:40:22Z

The github check looks good. I think for the UI, instead of having it in the notifications, there should be a bar on the top of the screen just sitting there all the time telling the user to update to a new version.

That was my first iteration but @buck54321 suggested we prefer notes.

See: #3286 (comment)

buck54321 · 2025-05-28T20:44:54Z

You guys win. Do the banner. Sorry for the back and forth.

martonp · 2025-05-29T16:25:27Z

That was my first iteration but @buck54321 suggested we prefer notes.

Lol I missed that completely.

JoeGruffins · 2025-06-05T07:44:24Z

client/webserver/site/src/html/bodybuilder.tmpl

@@ -12,6 +12,15 @@
  <link href="/css/style.css?v={{commitHash}}" rel="stylesheet">
 </head>
 <body class="dark{{if .UseDEXBranding}} dex-branding{{end}}">
+  {{if and (ne .Version .LatestVersion) .LatestVersion}}


What exactly is this comparison. Sorry.

if and is used for two comparisons, then those are, .Version != .LatestVersion and .LatestVersion existing at all?

I still think you should parse the version and probably do this somewhere else. It should compare the versions properly. Will this make our testing builds always hit?

Yes to your first question.

Nope, doesn’t make testing builds always hit…but we can take this check away from the html template and use a single bool variable.

JoeGruffins · 2025-06-05T07:45:14Z

client/webserver/webserver.go

+
+	for {
+		select {
+		case <-ticker.C:


I may still be missing something, but this could fire once without waiting, as soon as the app is started, then go into the loop if that fails.

multi: show upgrade banner if newer version is available on GitHub

cdf64ea

Signed-off-by: Philemon Ukane <[email protected]>

JoeGruffins reviewed May 23, 2025

View reviewed changes

client/webserver/webserver.go Outdated Show resolved Hide resolved

client/webserver/webserver.go Outdated Show resolved Hide resolved

JoeGruffins reviewed May 23, 2025

View reviewed changes

review changes

3ef07e0

Signed-off-by: Philemon Ukane <[email protected]>

redirect users to bison wallet website

e449ae8

Signed-off-by: Philemon Ukane <[email protected]>

ukane-philemon force-pushed the version-ui-notify branch from c92423f to e449ae8 Compare May 23, 2025 17:41

JoeGruffins reviewed May 24, 2025

View reviewed changes

client/webserver/webserver.go Show resolved Hide resolved

ukane-philemon changed the title ~~multi: show upgrade banner if newer version is available on GitHub~~ multi: notify user if newer version is available on GitHub May 24, 2025

JoeGruffins reviewed May 26, 2025

View reviewed changes

client/core/core.go Outdated Show resolved Hide resolved

JoeGruffins reviewed May 26, 2025

View reviewed changes

martonp reviewed May 28, 2025

View reviewed changes

ukane-philemon force-pushed the version-ui-notify branch from 568af47 to e449ae8 Compare May 29, 2025 01:17

ukane-philemon changed the title ~~multi: notify user if newer version is available on GitHub~~ multi: show upgrade banner if newer version is available on GitHub May 29, 2025

JoeGruffins reviewed Jun 5, 2025

View reviewed changes

		currentVersion := strings.Split(c.cfg.Version, "-")[0]
		if latestVersion > currentVersion && !strings.Contains(latestVersion, currentVersion) {

multi: show upgrade banner if newer version is available on GitHub #3286

Are you sure you want to change the base?

multi: show upgrade banner if newer version is available on GitHub #3286

Uh oh!

Conversation

ukane-philemon commented May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ukane-philemon May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

exitus1 commented May 23, 2025

Uh oh!

dev-warrior777 commented May 23, 2025

Uh oh!

exitus1 commented May 23, 2025

Uh oh!

ukane-philemon commented May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

buck54321 commented May 24, 2025

Uh oh!

ukane-philemon commented May 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JoeGruffins left a comment

Choose a reason for hiding this comment

Uh oh!

ukane-philemon commented May 26, 2025

Uh oh!

martonp left a comment

Choose a reason for hiding this comment

Uh oh!

ukane-philemon commented May 28, 2025

Uh oh!

buck54321 commented May 28, 2025

Uh oh!

martonp commented May 29, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ukane-philemon commented May 22, 2025 •

edited

Loading

ukane-philemon May 23, 2025 •

edited

Loading

ukane-philemon commented May 23, 2025 •

edited

Loading

ukane-philemon commented May 24, 2025 •

edited

Loading