100: Batched repository selection #119

katiefenn · 2017-02-10T11:50:19Z

Added initial support for batched snap creation to createSnap action
creators
Changed redirect on successful login to "select repositories"
dashboard view

katiefenn · 2017-02-10T11:53:33Z

src/common/actions/create-snap.js

@@ -54,10 +53,11 @@ function getSnapName(owner, name) {
    }));
 }

-export function createSnap(repositoryUrl, location) { // location for tests
+export function createSnap(repository) { // location for tests


I discovered that all code calling createSnap already had a parsed repository object. Instead of parsing it twice I decided to just pass the repository object.

katiefenn · 2017-02-10T11:54:36Z

src/common/actions/create-snap.js

-            if (result.status !== 'success' ||
-                result.payload.code !== 'snap-created') {
-              throw getError(response, result);
+            if (result.status !== 'success' || result.payload.code !== 'snap-created') {


Unrelated codestyle change.

katiefenn · 2017-02-10T11:56:59Z

src/common/actions/create-snap.js

-                result.payload.code !== 'snap-created') {
-              throw getError(response, result);
+            if (result.status !== 'success' || result.payload.code !== 'snap-created') {
+              return Promise.reject(getError(response, result));


createSnap is now called by createSnaps, so instead of handling the success and failure scenarios here they are now handled by the createSnaps promise.

katiefenn · 2017-02-10T12:04:11Z

src/common/actions/create-snap.js

        });
    }
  };
 }

+export function createSnaps(repositories, location) {


This action will eventually support batch creation of snaps. This is currently blocked in the LP auth API.

Until then, I've prepared this code to accept multiple repositories, but it actually only creates a snap for the first selected repository. This allows us work on the UI without breaking the staging website.

katiefenn · 2017-02-10T12:05:51Z

src/server/handlers/github-auth.js

@@ -66,7 +66,7 @@ export const verify = (req, res, next) => {
    logger.info('User successfully authenticated');

    // Redirect to logged in URL
-    res.redirect('/dashboard');
+    res.redirect('/dashboard/select-repositories');


After logging in, redirect to the dashboard select-repositories view as if logging in for the first time.

We don't technically need this change. Eventually, the user will only be redirected to the select-repositories view if they have no enabled repositories.

The reason I made this change now is that it will allow us to validate the "initial login" path before the smarter redirect is in-place. Our designers can see a more complete picture of the site before the "enabled-repositories" dashboard view is finished.

It's a small, meaningless change but I think it speaks a lot.

OK.

We probably need full workflow (with usable dashboard and enabled repos list and 'smarter' redirect) before it's meaningful to discuss the complete picture, but it doesn't hurt to have this for now.

cjwatson · 2017-02-10T12:22:57Z

src/common/actions/create-snap.js

@@ -54,10 +53,11 @@ function getSnapName(owner, name) {
    }));
 }

-export function createSnap(repositoryUrl, location) { // location for tests
+export function createSnap(repository) { // location for tests


The "location for tests" comment is now misplaced - it should be moved to createSnaps, since that's where the location parameter now lives.

Arg, nice spot 😅

cjwatson · 2017-02-10T12:24:46Z

src/common/components/select-repository-list/index.js

@@ -70,7 +70,7 @@ class SelectRepositoryList extends Component {
    const { selectedRepos } = this.props.selectRepositoriesForm;
    if (selectedRepos.length) {
      // TODO: Switch to batched repository action


This TODO comment is obsolete now, isn't it? That is, we still need changes inside createSnaps but not here.

- Added initial support for batched snap creation to createSnap action creators - Changed redirect on successful login to "select repositories" dashboard view

katiefenn commented Feb 10, 2017

View reviewed changes

katiefenn force-pushed the 100-batched-repo-integration branch from 13023c7 to d27a00f Compare February 10, 2017 12:14

katiefenn changed the title ~~WIP~~ 100: Batched repository selection Feb 10, 2017

cjwatson approved these changes Feb 10, 2017

View reviewed changes

katiefenn force-pushed the 100-batched-repo-integration branch from d27a00f to b5d3067 Compare February 10, 2017 12:32

100: Batched repository selection

eefde70

- Added initial support for batched snap creation to createSnap action creators - Changed redirect on successful login to "select repositories" dashboard view

katiefenn force-pushed the 100-batched-repo-integration branch from b5d3067 to eefde70 Compare February 10, 2017 12:35

katiefenn merged commit 5edb482 into master Feb 10, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

100: Batched repository selection #119

100: Batched repository selection #119

katiefenn commented Feb 10, 2017 •

edited

katiefenn Feb 10, 2017

katiefenn Feb 10, 2017

katiefenn Feb 10, 2017

katiefenn Feb 10, 2017

katiefenn Feb 10, 2017

katiefenn Feb 10, 2017

bartaz Feb 10, 2017

cjwatson Feb 10, 2017

katiefenn Feb 10, 2017

cjwatson Feb 10, 2017

100: Batched repository selection #119

100: Batched repository selection #119

Conversation

katiefenn commented Feb 10, 2017 • edited

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

katiefenn commented Feb 10, 2017 •

edited