Feature showing messages to the users, fixes #3799 #4885

gilbertsoft · 2023-05-08T11:22:10Z

The Issue

Create custom message/poll on ddev execution - similar to the new version warning - for use on user feedback #3799

We like to show messages to the user e.g. a hint to support DDEV with sponsoring the project.

How This PR Solves The Issue

This patch introduces an easy way to provide infos, warnings and rotating ticker messages in the remote-config repository which will be regulary downloaded and shown to the user.

Open tasks:

define a good first message about sponsoring DDEV
define some first tips

Manual Testing Instructions

To test this feature run a ddev command e.g. ddev describe, messages should be shown. Manipulate or delete the state file ~/.ddev/.state.yaml to get faster results. It's also possible to change the interval of the ticker in the ~/.ddev/global_config.yaml or switch to the testing branch of the remote-config, description to do so can be found in the config file.

Automated Testing Overview

Release/Deployment Notes

github-actions · 2023-05-08T11:27:32Z

Download the artifacts for this pull request:

See Testing a PR

docs/content/developers/messages.md

rfay · 2023-05-08T16:39:23Z

It would be cool to be able to show a variety of messages, so people don't get bored. Instructional, etc.

gilbertsoft · 2023-05-08T16:47:14Z

It would be cool to be able to show a variety of messages, so people don't get bored. Instructional, etc.

Nice idea. Like Docker Desktop does it on startup. Will think about...

gilbertsoft · 2023-05-11T08:03:39Z

It would be cool to be able to show a variety of messages, so people don't get bored. Instructional, etc.

Done

rfay

Taking a first casual look through. I don't understand it all yet, but made some suggestions about naming, etc. It's a lot of code for a small feature of course.

I see lots and lots of unexpected stuff being loaded into vendor, like github.com/ProtonMail (go-crypto I guess, but pulls in lots more, why that and also x/crypto?)? github.com/cloudflare (circl I guess, but what for?)? and of course the duplicated go-github/v52

Currently, every ddev start repeats the same message, "Ensure maintenance and further development of DDEV see..." We don't want to repeat the same message all the time, and don't want to show a message on every command. Maybe once a day?

It does seem to rotate on some of the other messages, but I don't think we want to show any message on most commands. Just daily or whatever?

But it's currently just on start, is that true?

manifest.json

docs/content/developers/manifest.md

pkg/github/github.go

pkg/manifest/manifest.go

rfay · 2023-05-11T17:24:54Z

pkg/manifest/manifest.go

+		m.Manifest, err = m.githubStorage.Pull()
+
+		if err != nil {
+			util.Error("Error while downloading manifest: %s", err)


I'm not sure we should be notifying the user about problems like this.

Yes, is currently mainly for testing purposes, Debug would be fine here too!

pkg/manifest/messages.go

pkg/manifest/storages/file_storage.go

rfay · 2023-05-11T17:31:36Z

pkg/manifest/storages/file_storage.go

+	data     fileStorageData
+}
+
+// fileStorageData is the structure used for the file.


If making a comment, at least give enough info to make it useful. Say what file, etc. Was this generated by Github Copilot?

A lot of comments will follow.... Also I like to have a description in each package what it is meant for.

pkg/manifest/storages/file_storage.go

gilbertsoft · 2023-05-12T11:08:57Z

Taking a first casual look through. I don't understand it all yet, but made some suggestions about naming, etc. It's a lot of code for a small feature of course.

I see lots and lots of unexpected stuff being loaded into vendor, like github.com/ProtonMail (go-crypto I guess, but pulls in lots more, why that and also x/crypto?)? github.com/cloudflare (circl I guess, but what for?)? and of course the duplicated go-github/v52

I've installed the latest version of go-github (v52) which is breaking and so far did not update the existing usages e.g. for ddev get or the update check. Imho we should do this later in a dedicated patch. So long, two version live together which does not hurt. The new version is also the reason for the new dependencies.

Currently, every ddev start repeats the same message, "Ensure maintenance and further development of DDEV see..." We don't want to repeat the same message all the time, and don't want to show a message on every command. Maybe once a day?

There are two features like mentioned in the docs: infos and warnings, which will show on every start, and the tips, which will rotate.

It does seem to rotate on some of the other messages, but I don't think we want to show any message on most commands. Just daily or whatever?

At the moment the messages are shown on ddev start. We should discuss how we like to have it.

The idea of infos and warnings is to have the posibility to show important messages to the user. This could make sense to be shown on most commands and on every call. E.g. if an update of an old version is important because of a security risk.

The tips is someting we also could implement to be shown once a day only, sounds good to me.

An ofc the messages should be printed in a nicer way, just did a simple output so far!

rfay · 2023-05-16T18:20:56Z

Requests from our meeting today:

Improve the upstream amplitude-go PR with an example of what could go wrong and more detail. We want them to understand it and pull it.
In the fork, https://github.com/ddev/analytics-go#readme, replace the README.md with an explanation of the reason for the fork, the link to the PR, etc. Deliver instrumentation data directly to amplitude, fixes #4335 #4866
Add the PR ink to the OP of this PR
(And please rebase, again :) )

gilbertsoft · 2023-05-16T19:48:57Z

Requests from our meeting today:

* [ ]   Improve the upstream amplitude-go PR with an example of what could go wrong and more detail. We want them to understand it and pull it.

* [x]  In the fork, https://github.com/ddev/analytics-go#readme, replace the README.md with an explanation of the reason for the fork, the link to the PR, etc. [Deliver instrumentation data directly to amplitude, fixes #4335 #4866](https://github.com/ddev/ddev/pull/4866)

* [x]   Add the PR ink to the OP of this PR

* [ ]  (And please rebase, again :) )

This belongs to #4866

gilbertsoft · 2023-05-17T11:36:11Z

@rfay what about remote config instead of manifest?

rfay · 2023-05-17T15:54:03Z

"remoteconfig" sounds OK to me.

I just realized that the time interval between showing messages should/could be specified in the manifest.json/remoteconfig.json (preferably remoteconfig.jsonc if we have a way to successfully read jsonc). Then we'd be able to introduce more important warnings, etc.

rfay · 2023-06-13T15:07:26Z

Rebased. Is this ready for review?

pkg/config/remoteconfig/messages.go

rfay · 2023-06-22T02:10:45Z

I haven't yet been able to make this work or get any kind of notification. We should do a screenshare to look at it tomorrow.

However, when it does work, it should absolutely not be creating any output except on ddev start IMO.

rfay · 2023-06-22T17:31:28Z

Yay, it worked, but we'll probably have to figure out something about wrapping.

rfay · 2023-06-22T17:42:37Z

Do you mind if I push this into the PR?

diff --git a/pkg/config/remoteconfig/messages.go b/pkg/config/remoteconfig/messages.go
index 605aaa54a..c3e649ba9 100644
--- a/pkg/config/remoteconfig/messages.go
+++ b/pkg/config/remoteconfig/messages.go
@@ -265,19 +265,27 @@ func applyTableStyle(preset preset, writer table.Writer) {
 		},
 	})
 
+	style := writer.Style()
+
+	style.Options.SeparateRows = false
+	style.Options.SeparateFooter = false
+	style.Options.SeparateColumns = false
+	style.Options.SeparateHeader = false
+	style.Options.DrawBorder = false
+
 	switch preset {
 	case information:
-		writer.Style().Color = table.ColorOptions{
+		style.Color = table.ColorOptions{
 			Header: text.Colors{text.BgHiYellow, text.FgBlack},
 			Row:    text.Colors{text.BgHiYellow, text.FgBlack},
 		}
 	case warning:
-		writer.Style().Color = table.ColorOptions{
+		style.Color = table.ColorOptions{
 			Header: text.Colors{text.BgHiRed, text.FgBlack},
 			Row:    text.Colors{text.BgHiRed, text.FgBlack},
 		}
 	case ticker:
-		writer.Style().Color = table.ColorOptions{
+		style.Color = table.ColorOptions{
 			Header: text.Colors{text.BgHiWhite, text.FgBlack},
 			Row:    text.Colors{text.BgHiWhite, text.FgBlack},
 		}

Result is

rfay · 2023-06-22T18:42:51Z

Per discussion in remote-images, let's go ahead and detect terminal width and wrap. I actually think the table code has this built in, and it will work fine for single-column stuff like this.

rfay · 2023-06-22T18:55:55Z

(Working on wrapping right now)

rfay · 2023-06-22T18:57:03Z

I think the remote config isn't created if it doesn't exist and the .state.yaml says that it might have been updated. I doubt this much matters. But it breaks testing.

rfay · 2023-06-22T18:57:30Z

I'm pretty sure these notes would be more effective at the beginning of ddev start instead of at the end.

This reverts commit 9d9a149.

rfay · 2023-06-22T21:15:16Z

I took the liberty to push a wrapping change in in e6bb8bf, we can revert if you don't like it. But here's what it looks like:

gilbertsoft · 2023-06-22T21:23:15Z

I took the liberty to push a wrapping change in in e6bb8bf, we can revert if you don't like it. But here's what it looks like:

Looks good, thanks!

rfay

Minor change requests, this seems to be working great. It's a really heavy load for what we expected, but I know you have great hopes for some of the things you added. Maybe we'll be able to use them where they can be used.

I tested with various terminal window sizes, and tested with internet disabled, and all seems to go well.

cmd/ddev/cmd/debug-message-conditions.go

docs/content/developers/remote-config.md

internal/build/remote_config.go

pkg/config/remoteconfig/config.go

rfay

Thanks for all the hard work on this, let's get it in. Please pull it when you're ready. Don't worry about the wsl failure, I'm sure it would clear, not related to this anyway.

gilbertsoft · 2023-06-24T19:44:48Z

Thanks for all the hard work on this, let's get it in. Please pull it when you're ready. Don't worry about the wsl failure, I'm sure it would clear, not related to this anyway.

Yes, had to restart the WSL test system, was completely broken.

gilbertsoft force-pushed the task/upstream-infos branch 2 times, most recently from be5a781 to 8346344 Compare May 8, 2023 11:51

gilbertsoft commented May 8, 2023

View reviewed changes

docs/content/developers/messages.md Outdated Show resolved Hide resolved

gilbertsoft force-pushed the task/upstream-infos branch from 8346344 to 43ebb2b Compare May 8, 2023 16:16

rfay changed the title ~~Feature showing messages to the users~~ Feature showing messages to the users, fixes #3799 May 8, 2023

gilbertsoft force-pushed the task/upstream-infos branch 2 times, most recently from 3c6640f to d1f8c9e Compare May 11, 2023 08:01

rfay requested changes May 11, 2023

View reviewed changes

gilbertsoft force-pushed the task/upstream-infos branch from 376da2b to 08429e1 Compare May 17, 2023 11:37

gilbertsoft force-pushed the task/upstream-infos branch 4 times, most recently from f0be104 to eae3d1b Compare May 19, 2023 17:46

gilbertsoft mentioned this pull request May 23, 2023

Introduce state management package to keep state of messages feature and global state #4948

Merged

gilbertsoft force-pushed the task/upstream-infos branch 5 times, most recently from 0ffb0f6 to 558c9f5 Compare May 25, 2023 08:51

rfay force-pushed the task/upstream-infos branch from a1ee990 to 9b016b7 Compare June 13, 2023 15:07

Remove trailing newline

023bcab

gilbertsoft force-pushed the task/upstream-infos branch from 1a354e4 to 023bcab Compare June 21, 2023 14:06

Use dedicated go files for new structs

305a053

gilbertsoft commented Jun 21, 2023

View reviewed changes

pkg/config/remoteconfig/messages.go Show resolved Hide resolved

gilbertsoft added 2 commits June 21, 2023 20:38

Doc fixes

6f0419b

Synchronous fetch from GH repo

f3652d2

gilbertsoft added 2 commits June 22, 2023 16:57

Fix condition and move notifications to start

c62ecd3

Avoid direct access to interval properties

142c3e4

Remove separator lines from messages

4dc7f8e

rfay added 3 commits June 22, 2023 14:12

Update go-pretty to 6.4.6

9d9a149

Soft-wrap text in message

e6bb8bf

Revert "Update go-pretty to 6.4.6"

5852779

This reverts commit 9d9a149.

gilbertsoft added 2 commits June 23, 2023 11:31

Fix docs

6bcdd92

Show messages at beginning of start

ca10070

rfay requested changes Jun 23, 2023

View reviewed changes

cmd/ddev/cmd/debug-message-conditions.go Show resolved Hide resolved

docs/content/developers/remote-config.md Outdated Show resolved Hide resolved

internal/build/remote_config.go Outdated Show resolved Hide resolved

pkg/config/remoteconfig/config.go Show resolved Hide resolved

gilbertsoft added 2 commits June 24, 2023 16:35

Move build to pkg

6dba459

Remove example config from docs and link to remote

5e84138

rfay approved these changes Jun 24, 2023

View reviewed changes

gilbertsoft merged commit 4dc214b into ddev:master Jun 24, 2023
16 of 17 checks passed

gilbertsoft deleted the task/upstream-infos branch June 24, 2023 20:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature showing messages to the users, fixes #3799 #4885

Feature showing messages to the users, fixes #3799 #4885

gilbertsoft commented May 8, 2023 •

edited

github-actions bot commented May 8, 2023 •

edited

rfay commented May 8, 2023

gilbertsoft commented May 8, 2023

gilbertsoft commented May 11, 2023

rfay left a comment

rfay May 11, 2023

gilbertsoft May 12, 2023

rfay May 11, 2023

gilbertsoft May 12, 2023

gilbertsoft commented May 12, 2023

rfay commented May 16, 2023 •

edited by gilbertsoft

gilbertsoft commented May 16, 2023

gilbertsoft commented May 17, 2023

rfay commented May 17, 2023

rfay commented Jun 13, 2023

rfay commented Jun 22, 2023

rfay commented Jun 22, 2023 •

edited

rfay commented Jun 22, 2023 •

edited

rfay commented Jun 22, 2023

rfay commented Jun 22, 2023

rfay commented Jun 22, 2023

rfay commented Jun 22, 2023

rfay commented Jun 22, 2023

gilbertsoft commented Jun 22, 2023

rfay left a comment

rfay left a comment •

edited

gilbertsoft commented Jun 24, 2023

Feature showing messages to the users, fixes #3799 #4885

Feature showing messages to the users, fixes #3799 #4885

Conversation

gilbertsoft commented May 8, 2023 • edited

The Issue

How This PR Solves The Issue

Manual Testing Instructions

Automated Testing Overview

Related Issue Link(s)

Release/Deployment Notes

github-actions bot commented May 8, 2023 • edited

rfay commented May 8, 2023

gilbertsoft commented May 8, 2023

gilbertsoft commented May 11, 2023

rfay left a comment

Choose a reason for hiding this comment

rfay May 11, 2023

Choose a reason for hiding this comment

gilbertsoft May 12, 2023

Choose a reason for hiding this comment

rfay May 11, 2023

Choose a reason for hiding this comment

gilbertsoft May 12, 2023

Choose a reason for hiding this comment

gilbertsoft commented May 12, 2023

rfay commented May 16, 2023 • edited by gilbertsoft

gilbertsoft commented May 16, 2023

gilbertsoft commented May 17, 2023

rfay commented May 17, 2023

rfay commented Jun 13, 2023

rfay commented Jun 22, 2023

rfay commented Jun 22, 2023 • edited

rfay commented Jun 22, 2023 • edited

rfay commented Jun 22, 2023

rfay commented Jun 22, 2023

rfay commented Jun 22, 2023

rfay commented Jun 22, 2023

rfay commented Jun 22, 2023

gilbertsoft commented Jun 22, 2023

rfay left a comment

Choose a reason for hiding this comment

rfay left a comment • edited

Choose a reason for hiding this comment

gilbertsoft commented Jun 24, 2023

gilbertsoft commented May 8, 2023 •

edited

github-actions bot commented May 8, 2023 •

edited

rfay commented May 16, 2023 •

edited by gilbertsoft

rfay commented Jun 22, 2023 •

edited

rfay commented Jun 22, 2023 •

edited

rfay left a comment •

edited