[AT] - updated ddev describe to strip credential information from out… #51

andrew-c-tran · 2017-11-15T22:17:52Z

…put, make links clickable and added tests.

The Problem/Issue/Bug:

#18 dictated how ddev describe should work in the UI, with changes to the original prototype code such as stripping command line specific info (mysql creds) from output and making links clickable

How this PR Solves The Problem:

functionality was added and tests written

Manual Testing Instructions:

run make test to build app and run tests

Related Issue Link(s):

#18

rfay

Am I mistaken or is this all using the plain-text user-oriented ddev output still? I'm confused why we'd do maintenance on that instead of moving to json parsing?
Clicking the link to mailhog, for example, results in the actual ddev-ui window loading mailhog. It's ugly. Don't we want it to load in a browser? I'm sure this isn't what we want, and of course there's no way back to ddev-ui.
The very most important information from describe is omitted here. We need this stuff right? At least the location and type and url.

NAME      TYPE     LOCATION              URL                         STATUS
typo3git  drupal7  ~/workspace/typo3git  http://typo3git.ddev.local  running

rfay · 2017-11-16T17:19:57Z

js/ddev-shell.js

                }else if(!currentLine){
                    currentSection = '';
                    inSection = false;
                }
            }
            resolve(siteDetails);
        }
-        ddevShell('describe', [siteName], null, parseDesribeLines, errorCallback, false);
+        ddevShell('describe', [siteName], null, parseDesribeLines, reject, false);


s/parseDesribeLines/parseDescribeLines/ - and where it is defined also

yes, you're right, this story was just updating the original code to have test coverage, i should go ahead and implement the json output. tagging as WIP and updating

andrew-c-tran · 2017-11-16T23:44:43Z

The links in the describe window were double opening, once in an external window and once inside the ddev-ui itself. this was unintentional, and has been corrected.

Name/Type/Location/URL were all intentionally omitted from this modal. The only way to access the DDEV describe modal is by clicking on the information button on the card for that site. The parent card already contains the site name (as does the header of the describe/info modal) as well as type, url, etc.

andrew-c-tran · 2017-11-17T01:36:24Z

Updated Pull to use json output of ddev describe (ddev describe sitename -j)
Updated tests accordingly
Fixed double open issue when clicking mailhog or phpmyadmin links
restored mysql credentials section; command line specific notes are omitted

…put, make links clickable and added tests.

…n from `ddev describe siteName -j` and updated tests accordingly

rfay

This version does not show any sites that can be described, so needs work.

rfay · 2017-11-17T20:55:23Z

test/ddev-shell-fixtures.js

@@ -8,6 +8,30 @@ DDEV ROUTER STATUS: running

 `;

+const validDescribeJSON = {


The real json that ddev-ui receives has no newlines embedded in it... Probably the fixture should be the same. To see what ddev-ui gets, try ddev describe -j | cat

The JSON that DDEV-UI is getting from the current library does have line breaks, the above output was taken directly from a breakpoint in the live DDEV-UI application itself. Full output is as follows (only using raw):

{ "level": "info", "msg": "NAME TYPE LOCATION URL STATUS \ndrupaltest drupal8 ~/Downloads/drupal http://drupaltest.ddev.local \u001b[36mrunning\u001b[0m\n\nMySQL Credentials\n-----------------\nUsername: \tdb \nPassword: \tdb \nDatabase name:\tdb \nHost: \tdb \nPort: \t3306\nTo connect to mysql from your host machine, use port 32768 on 127.0.0.1.\nFor example: mysql --host=127.0.0.1 --port=32768 --user=db --password=db --database=db\n\nOther Services\n--------------\nMailHog: \thttp://drupaltest.ddev.local:8025\nphpMyAdmin:\thttp://drupaltest.ddev.local:8036\n\nDDEV ROUTER STATUS: \u001b[36mhealthy\u001b[0m", "raw": { "approot": "~/Downloads/drupal", "dbinfo": { "dbname": "db", "host": "db", "password": "db", "port": "3306", "published_port": "32768", "username": "db" }, "mailhog_url": "http://drupaltest.ddev.local:8025", "name": "drupaltest", "phpmyadmin_url": "http://drupaltest.ddev.local:8036", "router_status": "healthy", "status": "running", "type": "drupal8", "url": "http://drupaltest.ddev.local" }, "time": "2017-11-17T14:21:00-07:00" }

Is ddev describe {site} -j not the correct command?

ddev only pretty-prints if it finds itself on a terminal. We can do that differently. But on commands other than describe (which is exactly one json object) you may not know when to start or stop parsing. Many emit various numbers of lines of json objects.

So either node is presenting a terminal emulation to ddev, or your debugger is pretty-printing for you (chrome debugger is happy to do that...), or it's possible we need to use another technique for ddev to make that decision. But I think you'll want one line per json object. Maybe it doesn't matter, but I'd expect it to.

ddev describe -j is the correct command. To see the output you would expect ddev-ui to consume, use ddev describe -j | cat, which is a pipe interface like ddev-ui should be seeing, with no pretty-printing.

you were right, it appears webstorm pretty prints the breakpoint output. corrected to be all one line

rfay · 2017-11-17T20:56:01Z

test/ddev-shell-fixtures.js

@@ -24,12 +48,15 @@ const expectedSitesArray = [
    }
 ];

+const expectedDescribeObject = {"MySQL Credentials":{"dbname":"db","host":"db","password":"db","port":"3306","published_port":"32768","username":"db"},"Other Services":{"MailHog":"<a onclick=\"electron.shell.openExternal('http://drupaltest.ddev.local:8025')\" href=\"#\">http://drupaltest.ddev.local:8025</a>","phpMyAdmin":"<a onclick=\"electron.shell.openExternal('http://drupaltest.ddev.local:8036')\" href=\"#\">http://drupaltest.ddev.local:8036</a>"}};


ddev-ui shouldn't be looking at this stuff, only at the 'raw' member.

expectedDescribeObject is the expected JSON output that's generated from the describe json from the CLI and passed to the view renderer, this is the correct format for it.

There is no json output with those hrefs and "Other services" in it... The raw part of the json output is like this:

"raw": { "approot": "/Users/rfay/workspace/randyfay", "dbinfo": { "dbname": "db", "host": "db", "password": "db", "port": "3306", "published_port": "32898", "username": "db" }, "httpsurl": "https://randyfay.ddev.local", "httpurl": "http://randyfay.ddev.local", "mailhog_url": "http://randyfay.ddev.local:8025", "name": "randyfay", "phpmyadmin_url": "http://randyfay.ddev.local:8036", "router_status": "healthy", "shortroot": "~/workspace/randyfay", "status": "running", "type": "drupal7" },

rfay · 2017-11-17T20:56:51Z

test/ddev-shell-tests.js

@@ -64,4 +63,21 @@ describe('ddev-shell', function () {
            ddevShell.restart('~/', function(){},function(){done()});
        });
    });
+    describe('#describe()', function () {
+        stubSpawnOnce('ddev describe drupaltest -j', 0, JSON.stringify(fixtures.validDescribeJSON));


I would really rather be using at least some non-stubbed ddev output. I understand the reason for using fixtures, but they're so limited.

rfay · 2017-11-17T20:57:17Z

test/ddev-shell-tests.js

+                    assert(error === fixtures.invalidDescribeOutput);
+                });
+        });
+    });
 });


Might as well make sure we have closing newlines. Sometimes not having them is a problem.

rfay

OK, let's get this in when the tests are green. Yay!

rfay · 2017-11-27T02:14:34Z

Yay!

rfay requested changes Nov 16, 2017

View reviewed changes

andrew-c-tran added the work in progress label Nov 16, 2017

andrew-c-tran removed the work in progress label Nov 17, 2017

andrew-c-tran added 4 commits November 16, 2017 23:12

[AT] - updated ddev describe to strip credential information from out…

b68e38f

…put, make links clickable and added tests.

[AT] - removed unnecessary test helper

f45ca26

[AT] - corrected double open of links in describe

1504b40

[AT] - removed string parsing of ddev-describe, added json consumptio…

4792a64

…n from `ddev describe siteName -j` and updated tests accordingly

rfay requested changes Nov 17, 2017

View reviewed changes

rfay mentioned this pull request Nov 17, 2017

package-lock.json gets rebuilt all the time, but tells us to check it in #27

Closed

andrew-c-tran added 3 commits November 20, 2017 14:52

[AT] - updated tests

38e4165

[AT] - updated list to use cli -j output and updated tests accordingly

2ca7901

[AT] - updated list to use cli -j output and updated tests accordingly

ea31059

rfay assigned andrew-c-tran Nov 21, 2017

rfay approved these changes Nov 21, 2017

View reviewed changes

[AT] - fixed tests, changed reference from atran to fakeuser in mockdata

d3b71a5

andrew-c-tran merged commit ef8ec5d into ddev:master Nov 27, 2017

andrew-c-tran mentioned this pull request Nov 27, 2017

Describe App #18

Closed

rickmanelius unassigned andrew-c-tran Mar 15, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[AT] - updated ddev describe to strip credential information from out… #51

[AT] - updated ddev describe to strip credential information from out… #51

andrew-c-tran commented Nov 15, 2017

rfay left a comment •

edited

Loading

rfay Nov 16, 2017

andrew-c-tran Nov 16, 2017

andrew-c-tran commented Nov 16, 2017

andrew-c-tran commented Nov 17, 2017

rfay left a comment

rfay Nov 17, 2017

andrew-c-tran Nov 17, 2017

rfay Nov 17, 2017

andrew-c-tran Nov 20, 2017

rfay Nov 17, 2017

andrew-c-tran Nov 20, 2017

rfay Nov 20, 2017

rfay Nov 17, 2017

rfay Nov 17, 2017

andrew-c-tran Nov 20, 2017

rfay left a comment

rfay commented Nov 27, 2017

		@@ -8,6 +8,30 @@ DDEV ROUTER STATUS: running

		`;

		const validDescribeJSON = {

[AT] - updated ddev describe to strip credential information from out… #51

[AT] - updated ddev describe to strip credential information from out… #51

Conversation

andrew-c-tran commented Nov 15, 2017

The Problem/Issue/Bug:

How this PR Solves The Problem:

Manual Testing Instructions:

Related Issue Link(s):

rfay left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrew-c-tran commented Nov 16, 2017

andrew-c-tran commented Nov 17, 2017

rfay left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rfay left a comment

Choose a reason for hiding this comment

rfay commented Nov 27, 2017

rfay left a comment •

edited

Loading