Issue 14 add from cms #62

andrew-c-tran · 2017-12-08T02:54:14Z

The Problem/Issue/Bug:

As a user, I need to be able to create a new site from a base CMS image

How this PR Solves The Problem:

Allows user to select a base CMS (drupal 7, 8, or wordpress), define the name and path, and start it.

Manual Testing Instructions:

make build darwin
open dist/ddev-ui-0.2.0-dev.dmg
run DDEV executable
click add site card in main view
enter credentials in pretty prompt so that we can modify hosts file
select any base image
give a valid site name * select a folder by clicking on the folder icon. 1

Related Issue Link(s):

#14

rickmanelius · 2017-12-12T17:21:12Z

To be clear, I'm going to step through manually testing before I would prefer that @rfay review at the code/test level to the degree that he can/feels comfortable with.

rickmanelius · 2017-12-12T17:27:04Z

Add App from Distribution (ddev-ui/issues/14)
- Opened ddev-ui.app from /dist/mac
- Clicked on the + in the upper right corner.
- Testing WordPress
  - App: WordPress
  - Name: JunkerWP
  - Install: ~/Desktop
  - Clicked "Create Site"
  - I see a spinner appear and the window greys out.
  - I am prompted to add my username and password.
  - I see activity go through with various status updates.
  - I see JunkerWP appear.
  - I can click and confirm a WordPress site is there.
  - I go to the terminal and can confirm a JunkerWP folder is there and a wordpress folder is within that.
- Testing Drupal 7
  - App: Drupal 7
  - Name: JunkerD7
  - Clicked "Create Site"
  - I see a spinner appear and the window greys out.
  - I am prompted to add my username and password.
  - I see activity go through with various status updates.
  - I see JunkerD7 appear.
  - I can click and confirm a Drupal 7 site is there.
  - I go to the terminal and can confirm a JunkerD7 folder is there and a "drupal-7.56" is within that.
- Testing Drupal 8
  - App: Drupal 8
  - Name: JunkerD8
  - Clicked "Create Site"
  - I see a spinner appear and the window greys out.
  - I am prompted to add my username and password.
  - I see activity go through with various status updates.
  - I see JunkerD7 appear.
  - I can click and confirm a Drupal 8 site is there.
  - I go to the terminal and can confirm a JunkerD8 folder is there and a "drupal-8.4.3" is within that.
  - I can confirm the vendor directory is populated
- CONFIRMED
  - Recommendation: Right now we're adding what I feel is an unnecessary parent folder. When a user specifies a folder, I would prefer that this is the parent folder and we're not pushing it one level inside the CMS name+version version (e.g. "drupal-8.4.3") folder.

rickmanelius

Added my full, manual review. Once @rfay reviews the test coverage and adds his feedback we can merge if you're ready @andrew-c-tran.

rickmanelius · 2017-12-12T22:45:24Z

Going to note this as a blocker because this issue allows us to close the v0.1 review and move onto other PRs, branches, issues, etc.

andrew-c-tran · 2017-12-16T06:04:20Z

Tests added, ready for code review.

rfay

Just a quick manual test.

I love the new finder option, looks so much better.
I started ddev-ui with no sites running, and it's just one big blank screen, with no prompts to do anything.
I'm pretty sure that when I manually started a site, the big blank screen remained big and blank the first time I went through this, but trying it out a second time I ended up with a very attractive screen. (Note that the router display is no longer there on the bottom, is that intentional?)
I removed the ~/.ddev/CMS to see what would happen. (Wasn't sure when the files were downloaded). That definitely didn't work out well. It results in a perpetual "Unzipping files" display. (Note that the "Create site" button remains active at this time.) Not sure if the whole thing is worth fixing, but ddev-ui certainly should be able to detect and handle an error of this type.
I created a drupal8 site successfully. However, the approot is not what I'd expect. I told it to put a site by the name of junker99.1 in ~/workspace, so I'd expect it to have an approot of ~/workspace/junker99.1, but it's actually ~/workspace/junker99.1/drupal-8.4.3. I think to solution to this is just to untar without keeping the top directory.
I note that there's no longer any option to remove a site.

The no-sites-running blank screen and the approot are certainly critical problems, along with the lack of error handling on unzipping, so some more work is needed on this. I haven't taken a look at the code yet.

…ot folder instead of target/wordpress(etc)

andrew-c-tran · 2017-12-18T23:25:04Z

addressed untar error handling, as well as approot path, pushed and build succeeded. looking into how to communicate no running sites now.

andrew-c-tran · 2017-12-18T23:52:33Z

@rfay the grey screen of no sites running is easily detectable from the output of ddev list -j (msg of There are no running ddev applications.) but what can we do from that point? The UI wouldn't have knowledge of what directories current sites live in to run ddev start in. Unless i'm overlooking something within ddev?

rfay · 2017-12-19T00:23:44Z

So @andrew-c-tran there's no reason not to have the "Add/Create Site" button when there are no sites running. That would make people feel more comfortable. And it should say somewhere "No sites are running". Otherwise, it just looks totally broken. Maybe "There are no sites running, create one!"

andrew-c-tran · 2017-12-19T00:27:49Z

Sure, we can definitely have just the add button, will have to figure out some very clear messaging for no running sites as I can see someone who has only used the UI wondering where their sites went. Even then, if they didn't want to create a new site, how would one with no CLI experience (or the knowledge that a CLI even exists for that matter) start a site? On Dec 18, 2017 5:23 PM, "Randy Fay" <[email protected]> wrote: So @andrew-c-tran <https://github.com/andrew-c-tran> there's no reason not to have the "Add/Create Site" button when there are no sites running. That would make people feel more comfortable. And it should say somewhere "No sites are running". Otherwise, it just looks totally broken. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#62 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AIi5a3jVJ5bhssGRwBGBHvITDWIqHMj3ks5tBwIRgaJpZM4Q6g6r> .

rfay · 2017-12-19T00:29:25Z

I think the (current) inability to create a site from within the ddev-ui is a serious problem, because people who want to try ddev-ui will be baffled at the start.

rfay · 2017-12-19T02:55:04Z

Not having any sites running is absolutely not an error.

Now when I start with no sites running I get a perpetual set of big red errors:

And then when I start a site, I have this:

rfay · 2017-12-19T03:51:19Z

The error checking is better, there are still a lot of problems.

The error overlay should be opaque I'm pretty sure. It's pretty hard to read.

If you create an error (like don't select a CMS type, but just select the location on disk and name, there's no way to recover because it's all kept in state. If you use the ESC key to back out of the error (there's no other way, we seem to just hope they can do it), and then try again, the error already exists. You can't recover from the error except by exiting ddev-ui.
You seem to have accidentally checked in the mainWindow.openDevTools();. I think we probably need a better way to manage that than a line of code, because I'm sure this has happened more than once before.
It's not quite clear what action causes the finder-select window to pop up. (It does seem to be the folder icon, now that I experiment with it more. But I clicked a lot of times and a lot of things to figure out. It's not at all clear that that's a button.

I haven't looked at the code yet, just fiddled with ddev-ui a bit.

andrew-c-tran · 2017-12-20T00:26:07Z

updated to show add new site card when no sites running with an info message in footer. Note this status will only show with non-healthy router-statuses as per design discussion with @rickmanelius
Error screen updated for readability and dismissal, and auto clearing of errors upon dismissal of modal. I wouldn't put too much concern into the look and feel of this error screen at the moment as there are style guide changes regarding errors and soon modals that are in flight in the styleguide (Error and loading states drud/style.ddev.com#4 and Standardized Modal Structure drud/style.ddev.com#2)
Removed the manual entry option for site path; the entire label now opens file browser to avoid confusion.

rfay · 2017-12-20T14:01:51Z

I see the router monitoring has been reintroduced (and no sites running notification), and it often works. However, it doesn't work if you start ddev-ui with no sites running and then start some sites. It continues to insist forever that the router's not running and there are no sites.

No sites running
Start ddev-ui
Use ddev to start a site
Warning in lower right stays forever.

rfay

I didn't get a chance to look at the tests yet but wanted to finish this here. Lots of comments, some that you can do things about now, others that you probably can't.

The cold-start with no sites now looks better, thanks! At least you can tell there's something to do.

Router status and "no sites" doesn't work right if you start with no sites and then start a site. It continues to say there aren't any sites.
The security implications of your use of sudo are off the chart, that needs to be rethought carefully.
Windows doesn't seem to be handled or tested at this point, and I'm pretty sure we're aiming for windows to work all the time still right?
Please add function headers to all code, not just the new stuff in this PR. (ddev-shell.sh and ui.js don't have anything).
Markup embedded in js with explicit style= doesn't seem right. Why isn't CSS being used for that stuff?

rfay · 2017-12-20T14:06:35Z

index.html

@@ -50,6 +52,29 @@ <h2 class="modal-title" id="modalLabel">Modal title</h2>
        </div>
    </div>

+    <div class="modal fade" id="addOptionsDialog" tabindex="-1" role="dialog" aria-labelledby="addOptionsDialog" aria-hidden="true">


I guess I would expect this to be in the js application and not in the index.html?

removed modals and created a createModal template that programmatically generates and injects modals into the DOM.

rfay · 2017-12-20T14:15:13Z

js/cms-installer.js

+function validateCMSType(cmsType){
+    var promise = new Promise(function(resolve, reject){
+        var cmsString = cmsType.toLowerCase();
+        if(cmsString === 'wordpress' || cmsString === 'drupal7' || cmsString === 'drupal8'){


This type of code is of course what we're ripping out of ddev, because we're going to support a number of CMSs. This would be a lot nicer as a check against an array of possible types though (instead of inline if statement), probably configured globally. I don't think there's any need to do that yet, but I hate to see us have to do all the work we're doing now in ddev to generalize this. So I'm just giving warning, but probably nothing needs to be done here yet. Opened #67 for this.

rfay · 2017-12-20T14:17:08Z

js/cms-installer.js

+                targetCMS = 'wordpress';
+                break;
+            case 'drupal7':
+                targetCMS = 'drupal-7';


Life would be a lot better if cmsType and targetCMS used the same semantics. Or use an associative array to map instead of a switch. No action needed here, but it would really be a lot nicer.

So what I was saying here is, if possible, use the same words for targetCMS as you do for cmsType. Drop the drupal-7 stuff and just use drupal7.

rfay · 2017-12-20T14:19:26Z

js/cms-installer.js

+ */
+function unpackCMSTarball(tarballPath, outputPath) {
+    var promise = new Promise(function(resolve,reject){
+        exec('mkdir '+outputPath, function(err, stdout, stderr) {


Isn't there a better way to create a directory than execing mkdir? Unless I'm misunderstanding, this should be done another way. Does Windows mkdir even work the same?

can subsitute mkdir with node's fs.mkdir that will use a platform specific call. Good catch. node-tar does require an existing, writable path though, so I do believe creating the directory via fs.mkdir is the only option here.

rfay · 2017-12-20T14:23:57Z

js/cms-installer.js

+ * @param siteName {string} name of site to create hosts file entry
+ * @return {promise} resolves with a successful terminal output from ddev hostname, rejects with ddev error output
+ */
+function updateHostsFile(siteName) {


This sure doesn't seem to be a very useful function. Why not just call ddevShell.hostname()? It also doesn't return a promise does it?

yes, ddevShell.hostname() does indeed return a promise, and there is no reason for this wrapper. removed.

rfay · 2017-12-20T14:47:02Z

js/ui.js

+            </div>
+            <div class="card-body">
+                <a href="#">
+                    <div style="height: 155px; line-height: 155px; font-size: 75px;" >


Shouldn't this be in the css instead of use style here?

all inline CSS removed and pulled into stylesheet

rfay · 2017-12-20T14:47:19Z

js/ui.js

+                    </div>
+                </a>
+            </div>
+            <div style="height:73px;" class="card-footer">


In css instead of in style=?

all inline CSS removed and pulled into stylesheet

rfay · 2017-12-20T14:51:43Z

js/ui.js

+        var options = {
+            name: 'DDEV UI',
+        };
+        var command = 'ls';


This can't possibly work on Windows, right? Since there's no ls command?

You're also execing 'ls' with sudo, without controlling the path. So all I have to do is write a script called ls that does rm -rf / and it will destroy the machine? This needs to be seriously rethought.

I'm pretty sure that ddev needs to be accessed always with a full configured path. If we have a reason to trust ddev, then ddev version or just ddev would be a reasonable command here (with full path of trusted ddev). But just assuming a random command like ls isn't going to be very good.

rfay · 2017-12-20T14:54:17Z

js/ui.js

        $('#distroModal').modal();
    });
+    $(document).on('click', '.start-from-directory', function () {
+        alert('WIP...');


Needs to have something real right?

This was a fragment, should not have been here and was removed

rfay · 2017-12-20T14:55:02Z

main.js

@@ -25,7 +25,7 @@ app.on('ready', function () {
    mainWindow.loadURL('file://' + __dirname + '/index.html');

    // Open the DevTools.
-    mainWindow.openDevTools();    // requires a height 410px
+    // mainWindow.openDevTools();    // requires a height 410px


Let's get this controlled with a different technique (global config?) instead of commenting and uncommenting code.

added detection if running in development mode (via electron . in cli) vs production (compiled/built binary) and will launch devtools if in dev mode.

rickmanelius · 2017-12-26T17:27:19Z

I had a chance to review this with @andrew-c-tran this morning where we discussed this and other ddev-ui priorities and the overall roadmap. The rough draft notes can be found here https://docs.google.com/document/d/1Xy0ag25AA9NqmH6MhLIph5abcp9i_YD5mG7F5RS14jQ/edit#. I think these are the most pertinent items to get this issue to the finish line while moving other issues to an appropriate area.

Design Issues: This has been a pain point that needs its own set of attention because design (wireframes, comps, etc) has been a shared responsibility among 4 people (@andrew-c-tran, @CRReigan, @cyberswat, and @rickmanelius) and that is resulting in quite a bit of thrashing. To that end, I'm going to split out a new milestone in the roadmap right after v0.1 and v0.2 to inventory all necessary items (iconography, transition states for errors/progress, etc) and to move current assets upstream into the style guide. We will then adopt a process moving forward where all requests for design assets get filed, implemented, reviewed, and committed here https://github.com/drud/style.drud.com as a dependency.
Remaining Functionality: The big remaining open loop for @andrew-c-tran is the sudo security implication that @rfay noted. There are other important items to address (such as moving mainWindow.openDevTools() to a global config) and that should be split out as a separate issue because it's a separate bit of functionality apart from adding a CMS.
Remaining Review: While @rfay has done a superb job of reviewing the end-user functionality as well as a code review, we still need a review on tests. To that end, I would still like @rfay or @cweagans to complete a final pass on this once the security functionality is completed.
v0.1 review. Once items 1. is provisioned on the roadmap and in the issue queue and items 2 and 3 are completed, I'll complete the end-user review so that we can approve and release v0.1.

I concur there are some open issues on design/colors/styles, but I want to make sure we're scoping properly and we will prioritize this in v0.3 given that much of the functionality for v0.1-v0.2 is in flight and I'd like to reduce WIP as much as possible with the hope that we have a more structured cadence post v0.3.

BTW, if my summary is wrong or missing any key items, I'm happy to have a conversation. However, if there are no objections, I'd like to keep this as the path forward.

…x to dynamically generated, guarded sudo

…etection for devtools

…e easier for testing

andrew-c-tran · 2017-12-28T05:14:14Z

@rfay 's comments addressed, cleanup of code, and tested (including priv escalation) in windows 10 pro and ubuntu Ubuntu 16.04.3 LTS

rfay

Some things for you to work on while I get to the code review. The first is highly critical IMO.

If you try to create a site in a directory that already exists, It will spin in the "unzipping files" forever, the error is never notified. Please improve error handling on this process. This is a critical error, and I would imagine there are other ways to demonstrate it. We need to catch errors like this. (This situation isn't even an error, there's nothing really wrong with creating a site in an empty existing directory.) You get the same result if the chosen directory has permissions that don't allow creation of a subdirectory. For example, mkdir /tmp/j; chmod 444 /tmp/j" then use /tmp/j as the directory and junker99 as the sitename.
I see the code still has Start Process Initiated. It may take a few seconds for the new site card to appear - Please improve that as our users don't know what a "site card" is. Maybe just "site" would do the job, drop "card".
The errors "hostname cannot be blank" and "hostname is invalid" should read "Project name cannot be blank", etc.

rfay

Made several smaller comments; can't finish the code today. I got down to about #62 (comment), can start there Tuesday.

rfay · 2017-12-28T18:15:04Z

js/bootstrap-modal.js

+ * @returns {string} - markup for a boostrap modal
+ */
+function createModal(id, title, body, footer) {
+    var markup = `<div class="modal fade" id="`+id+`" tabindex="-1" role="dialog" aria-labelledby="`+id+`Label" aria-hidden="true">


Just wish this was already jsx.

rfay · 2017-12-28T18:16:32Z

js/cms-installer.js

+    $('.loading-overlay').css('display', displayType);
+}
+
+function showErrorScreen(display, error='Something Went Wrong'){


I think you were intending (I think) to do function headers everywhere. These can use them IMO.

Correct, i missed those two. Minor refactor to make them more consistent with one another, and added JSDOC headers

rfay · 2017-12-28T18:32:40Z

js/cms-installer.js

+        if(hostnameRegex.test(hostname.toLowerCase())){
+            resolve(true);
+        } else {
+            var error = hostname ? 'Hostname is Invalid.' : 'Hostname Cannot Be Blank.';


Since we don't expose this to them as a hostname, but rather as "project", this should be adjusted to match what we're presenting to them in the gui.

updated to "project name" vs "hostname"

rfay · 2017-12-28T18:37:47Z

js/cms-installer.js

+        if(cmsString === 'wordpress' || cmsString === 'drupal7' || cmsString === 'drupal8'){
+            resolve(true);
+        } else {
+            var error = cmsType ? 'CMS Type is Invalid.' : 'CMS Type Cannot Be Blank.';


This is kind of an ugly message "CMS type cannot be blank" as they didn't type anything. Better "Please select a CMS type".

updated to "please select a cms type"

rfay · 2017-12-28T18:46:15Z

js/cms-installer.js

+                targetCMS = 'wordpress';
+                break;
+            case 'drupal7':
+                targetCMS = 'drupal-7';


So what I was saying here is, if possible, use the same words for targetCMS as you do for cmsType. Drop the drupal-7 stuff and just use drupal7.

rfay · 2017-12-28T18:47:21Z

js/cms-installer.js

+                    resolve(cmsPath + '/' + fileName);
+                }
+            });
+            reject('CMS tarball not found');


"CMS archive not found" is friendlier.". I'd say also give a mention of where it was looking for the tarball, because if we have a user with this problem we'll be able to help them better.

rfay · 2017-12-28T18:47:47Z

js/cms-installer.js

+            });
+            reject('CMS tarball not found');
+        }).catch(function(){
+            reject('CMS tarball not found in `~/.ddev/CMS`. Restarting the UI will attempt to redownload these files.');


This reject verbiage doesn't happen any more does it?

ah good catch (pun intended). the catch never fires in this case, i've removed it and changed the reject message above to "CMS archive not found in ~/.ddev/CMS. Restarting the UI will attempt to redownload these files."

rfay · 2017-12-28T18:49:58Z

js/cms-installer.js

+ * @param targetFolder {string} target folder to extract to
+ * @return {promise} resolves with path to new site docroot, rejects with error returned from any failed called function
+ */
+function createFiles(siteName, cmsType, cmsPath, targetFolder){


This seems like a pretty wimpy function name given the job it performs. Improving it will improve readability.

definitely understated it's functionality. i've made a shot at updating it to be more accurate

rfay · 2018-01-02T18:59:33Z

js/cms-installer.js

+function addCMS(name, type, targetPath) {
+    var cmsPath = "~/.ddev/CMS";
+    var workingPath = cmsPath;
+    cmsPath = cmsPath.replace('~', os.homedir());


Don't you have to do this more than one place? It should probably be a global utility function. We sure have to do it a lot more than once in ddev.

rfay · 2018-01-02T19:00:21Z

js/cms-installer.js

+    .then((stdout) => {
+        if(stdout.toString().indexOf('Starting environment') != -1){
+            resetAddModal();
+            alert('Start Process Initiated. It may take a few seconds for the new site to appear on your dashboard.');


Like the new verbiage.

rfay · 2018-01-02T19:01:57Z

js/cms-installer.js

+        showLoadingScreen(true,'Updating Hosts File');
+        return ddevShell.hostname(name);
+    })
+    .then(() => {


This whole section works pretty well. Not sure about the modal dialog at the end, but that will be talked about down the line.

rfay · 2018-01-02T19:05:06Z

js/cms-installer.js

+        resetAddModal();
+        alert('In order to add a new site, DDEV requires elevated permissions to modify your Hosts file. You may be prompted for your username and password to continue.');
+        var command = 'version';
+        ddevShell.sudo(command)


sudo-ing ddev version is pretty safe, but still a bit odd. I assume you're trying to get the sudo out of the way and hoping it won't be needed for ddev hostname. I'm not sure this needs to be done here as opposed to later when we know the sitename.

ddev version is the only command allowed to initiate a permissions request dialog, and yes it is to get sudo out of the way as well as ensuring that the add flow can actually finish before having the user go through and select a CMS, directory, unpack it all, only to find it fails during hostname setup for some sudo reason. It's super benign and there's no risk in running it multiple times in succession if a user should open and cancel out of the add site flow multiple times in succession. ddev hostname then inherits sudo from this as you guessed.

thinking about this further, perhaps we should also allow ddev hostname to prompt in the event a user doesn't have tty_tickets enabled

I think you're assuming that sudo is set up to have a timeout like a period of time where pass won't be required. This is not necessarily the case. Sudo is often set up to always require password.

But my question was really, why would you have a whitelist that didn't include the most important command (hostname)? And if it doesn't include that, is it working at all?

I was under the assumption that tty_ticket timestamping would allow the end user a grace period in which they didn't have to re-enter their password? In documentation and some of my tests it appeared that on osx that grace is 5 minutes and on ubuntu it's 15 by default. I know this is something that can be changed, though, and perhaps it's unwise to assume it's enabled by default on everyone's machine.

The intention for the whitelist was to catch non-intended escalation requests, and to fail out if a sudo dialog flow was started from any other entry point. We can filter all commands through this if that's the end goal. In hindsight, that's probably the better route, to corral every command through this screen, and not just the entry points

ah, i see, the whitelist can and should be abstracted to a higher level and cut off the sudo library in it's entirety, will open a pull and do that now

rfay · 2018-01-02T19:05:45Z

js/cms-installer.js

+                alert(err);
+            });
+    });
+    $(document).on('click', '.start-from-template', function () {


Looking forward to React handling for this stuff.

rfay · 2018-01-02T19:11:49Z

js/ddev-shell.js

+ */
+const sudo = (command, promptOptions = {name: 'DDEV UI'}) => {
+    var bannedCharacters = [';','|','&'];
+    var whitelistedCommands = ['version'];


Doesn't hostname have to be whitelisted for this to work? What's up with that?

oh, dangit, i got so ahead of myself looking forward that i completely missed what you were actually saying. I see now, and You're right, will address sudo in a much more thoroughly tested and protected way and address a flow issue relevant to both .1 and .2

rfay · 2018-01-02T19:20:29Z

js/ddev-shell.js

@@ -58,7 +59,12 @@ const list = () => {
    var promise = new Promise((resolve, reject) => {
        function getRaw(output) {
            var outputObject = JSON.parse(output);
-            if(Array.isArray(outputObject.raw)){
+            if(outputObject.msg === 'There are no running ddev applications.') {


Looking for info instead of array is probably ok. Would rather ddev do something saner. Not exactly sure how to pull that off. Opened ddev/ddev#588

rfay

Thanks for all the work on this! It looks better and is more robust than ever before.

I'm ok with this now, although I haven't tested on Linux or Windows.

Please take a look at these few comments, I think the whitelisting of sudo commands isn't working as expected?

andrew-c-tran added 2 commits December 7, 2017 20:26

[AT] - WIP initial commit - add from cms working.

2e8989a

[AT] - WIP initial commit - add from cms working.

c204047

andrew-c-tran added the work in progress label Dec 8, 2017

[AT] - WIP - updated add flow

7e2cb1e

rickmanelius requested review from rfay and rickmanelius December 12, 2017 17:20

rickmanelius removed the request for review from rfay December 12, 2017 17:21

[AT] - WIP - restored accidentally removed spinner

dd1f72c

rickmanelius approved these changes Dec 12, 2017

View reviewed changes

rickmanelius added the blocker label Dec 12, 2017

andrew-c-tran added 3 commits December 14, 2017 11:08

[AT] - visual updates to accordions

cea41fe

[AT] - added JSDOC comments to functions

b3d71d3

[AT] - added full tests

48de384

andrew-c-tran removed blocker work in progress labels Dec 16, 2017

rfay requested review from cweagans and rfay December 18, 2017 15:45

rfay requested changes Dec 18, 2017

View reviewed changes

andrew-c-tran added 3 commits December 18, 2017 13:04

[AT] - wip - pull notes fix

942d1ea

[AT] - added handling for tarball missing error, unpacked files to ro…

fcfd443

…ot folder instead of target/wordpress(etc)

[AT] - updated completion messaging

9f83ba0

andrew-c-tran added 2 commits December 19, 2017 17:21

[AT] - updated error screen styles, added support for no router running

0cb990e

[AT] - updated error screen styles, added support for no router running

dbc04d0

andrew-c-tran mentioned this pull request Dec 20, 2017

[AT] - added remove site modal, controller, and tests #64

Merged

rfay requested changes Dec 20, 2017

View reviewed changes

rickmanelius mentioned this pull request Dec 26, 2017

Add app from CMS distribution #14

Closed

andrew-c-tran mentioned this pull request Dec 26, 2017

Router Status Reporting #41

Closed

andrew-c-tran added 8 commits December 26, 2017 15:02

[AT] - removed inline styles, moved modals from static markup in inde…

d11941e

…x to dynamically generated, guarded sudo

[AT] - flattened "promise pyramid"

f4a89e6

[AT] - updated no sites detection

38654f5

[AT] - updated to use fs.mkdir for cross platform compatability

da57ba9

[AT] - added JSDOC documentation to ddev-shell.js and added dev-env d…

56f581f

…etection for devtools

[AT] - module extraction, added JSDOC comments for ALL code, made lif…

4b1ffc0

…e easier for testing

[AT] - updated comment and company name

40bd0b1

[AT] - updated test comments

468da71

rfay requested changes Dec 28, 2017

View reviewed changes

andrew-c-tran added 2 commits December 28, 2017 12:20

[AT] - addressed several feedback changes

7ccbdf6

[AT] - fixed tests

d6f61aa

rfay mentioned this pull request Jan 2, 2018

ddev list -j should/could return empty array (and router status) when no sites running ddev/ddev#588

Closed

rfay reviewed Jan 2, 2018

View reviewed changes

rfay approved these changes Jan 2, 2018

View reviewed changes

andrew-c-tran merged commit 3bb6906 into ddev:master Jan 2, 2018

Issue 14 add from cms #62

Issue 14 add from cms #62

Conversation

andrew-c-tran commented Dec 8, 2017 • edited Loading

The Problem/Issue/Bug:

How this PR Solves The Problem:

Manual Testing Instructions:

Related Issue Link(s):

rickmanelius commented Dec 12, 2017

rickmanelius commented Dec 12, 2017 • edited Loading

rickmanelius left a comment

Choose a reason for hiding this comment

rickmanelius commented Dec 12, 2017

andrew-c-tran commented Dec 16, 2017

rfay left a comment

Choose a reason for hiding this comment

andrew-c-tran commented Dec 18, 2017

andrew-c-tran commented Dec 18, 2017

rfay commented Dec 19, 2017 • edited Loading

andrew-c-tran commented Dec 19, 2017 via email

rfay commented Dec 19, 2017

rfay commented Dec 19, 2017

rfay commented Dec 19, 2017

andrew-c-tran commented Dec 20, 2017 • edited Loading

rfay commented Dec 20, 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

rfay Dec 20, 2017 • edited Loading

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

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

rickmanelius commented Dec 26, 2017

andrew-c-tran commented Dec 28, 2017

rfay left a comment

Choose a reason for hiding this comment

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

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

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

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

andrew-c-tran commented Dec 8, 2017 •

edited

Loading

rickmanelius commented Dec 12, 2017 •

edited

Loading

rfay commented Dec 19, 2017 •

edited

Loading

andrew-c-tran commented Dec 20, 2017 •

edited

Loading

rfay Dec 20, 2017 •

edited

Loading

rfay left a comment •

edited

Loading