Skip to content
This repository has been archived by the owner on Jun 11, 2020. It is now read-only.

100: Batched repository selection #119

Merged
merged 1 commit into from Feb 10, 2017
Merged

100: Batched repository selection #119

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
55 changes: 35 additions & 20 deletions src/common/actions/create-snap.js
View file
Open in desktop
Expand Up @@ -2,7 +2,6 @@ import 'isomorphic-fetch';

import { checkStatus, getError } from '../helpers/api';
import { conf } from '../helpers/config';
import { parseGitHubRepoUrl } from '../helpers/github-url';

const BASE_URL = conf.get('BASE_URL');

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

export function createSnap(repositoryUrl, location) { // location for tests
export function createSnap(repository) {
return (dispatch) => {
const repositoryUrl = repository.url;
if (repositoryUrl) {
const { owner, name, fullName } = parseGitHubRepoUrl(repositoryUrl);
const { owner, name, fullName } = repository;

dispatch({
type: CREATE_SNAP,
Expand All @@ -81,30 +81,45 @@ export function createSnap(repositoryUrl, location) { // location for tests
.then(checkStatus)
.then(response => {
return response.json().then(result => {
if (result.status !== 'success' ||
result.payload.code !== 'snap-created') {
throw getError(response, result);
if (result.status !== 'success' || result.payload.code !== 'snap-created') {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated codestyle change.

return Promise.reject(getError(response, result));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}
const startingUrl = `${BASE_URL}/${fullName}/setup`;
(location || window.location).href =
`${BASE_URL}/login/authenticate` +
`?starting_url=${encodeURIComponent(startingUrl)}` +
`&caveat_id=${encodeURIComponent(result.payload.message)}` +
`&repository_url=${encodeURIComponent(repositoryUrl)}`;

return Promise.resolve(result);
});
})
.catch(error => {
// if LP error says there is already such snap, just redirect to builds page
if (error.message === 'There is already a snap package with the same name and owner.') {
(location || window.location).href = `${BASE_URL}/${fullName}/builds`;
} else {
dispatch(createSnapError(fullName, error));
}
});
}
};
}

export function createSnaps(repositories, location) { // location for tests
return (dispatch) => {
// Patch the creation of multiple snaps
// until auth supports it. Until then,
// only process the first selected repo
repositories = [ repositories[0] ];
const { fullName, url } = repositories[0];

const promises = repositories.map((repository) => dispatch(createSnap(repository)));
return Promise.all(promises)
.then((results) => {
// Redirect to Launchpad auth for the first
// selected repo until auth supports batched
// creation of snaps
const startingUrl = `${BASE_URL}/${fullName}/setup`;

(location || window.location).href =
`${BASE_URL}/login/authenticate` +
`?starting_url=${encodeURIComponent(startingUrl)}` +
`&caveat_id=${encodeURIComponent(results[0].payload.message)}` +
`&repository_url=${encodeURIComponent(url)}`;
})
.catch(error => {
dispatch(createSnapError(fullName, error));
});
};
}

export function createSnapError(id, error) {
return {
type: CREATE_SNAP_ERROR,
Expand Down
5 changes: 2 additions & 3 deletions src/common/components/select-repository-list/index.js
View file
Open in desktop
Expand Up @@ -2,7 +2,7 @@ import React, { Component, PropTypes } from 'react';
import { connect } from 'react-redux';

import { conf } from '../../helpers/config';
import { createSnap } from '../../actions/create-snap';
import { createSnaps } from '../../actions/create-snap';
import { toggleRepository } from '../../actions/select-repositories-form';
import SelectRepositoryRow from '../select-repository-row';
import Spinner from '../spinner';
Expand Down Expand Up @@ -69,8 +69,7 @@ class SelectRepositoryList extends Component {
onSubmit() {
const { selectedRepos } = this.props.selectRepositoriesForm;
if (selectedRepos.length) {
// TODO: Switch to batched repository action
this.props.dispatch(createSnap(selectedRepos[0].url));
this.props.dispatch(createSnaps(selectedRepos));
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/server/handlers/github-auth.js
View file
Open in desktop
Expand Up @@ -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');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

});
};

Expand Down
4 changes: 2 additions & 2 deletions test/routes/src/server/routes/t_github-auth.js
View file
Open in desktop
Expand Up @@ -105,14 +105,14 @@ describe('The login route', () => {
);
});

it('should redirect request to dashboard', (done) => {
it('should redirect request to the dashboard select repositories view', (done) => {
supertest(app)
.get('/auth/verify')
.query({ code: 'foo', state: 'bar' })
.send()
.end((err, res) => {
expect(res.statusCode).toEqual(302);
expect(res.headers.location).toEqual('/dashboard');
expect(res.headers.location).toEqual('/dashboard/select-repositories');
done(err);
});
});
Expand Down
17 changes: 11 additions & 6 deletions test/unit/src/common/actions/t_create-snap.js
View file
Open in desktop
Expand Up @@ -6,7 +6,7 @@ import { isFSA } from 'flux-standard-action';
import url from 'url';

import {
createSnap,
createSnaps,
createSnapError,
setGitHubRepository
} from '../../../../../src/common/actions/create-snap';
Expand Down Expand Up @@ -57,8 +57,13 @@ describe('repository input actions', () => {
});
});

context('createSnap', () => {
const repositoryUrl = 'https://github.com/foo/bar';
context('createSnaps', () => {
const repository = {
url: 'https://github.com/foo/bar',
fullName: 'foo/bar',
owner: 'foo',
name: 'bar'
};
const BASE_URL = conf.get('BASE_URL');
let scope;

Expand All @@ -81,7 +86,7 @@ describe('repository input actions', () => {
});
scope
.post('/api/launchpad/snaps', {
repository_url: repositoryUrl,
repository_url: repository.url,
snap_name: 'test-snap',
series: '16',
channels: ['edge']
Expand All @@ -95,7 +100,7 @@ describe('repository input actions', () => {
});

const location = {};
return store.dispatch(createSnap(repositoryUrl, location))
return store.dispatch(createSnaps([ repository ], location))
.then(() => {
expect(url.parse(location.href, true)).toMatch({
path: '/login/authenticate',
Expand All @@ -118,7 +123,7 @@ describe('repository input actions', () => {
});

const location = {};
return store.dispatch(createSnap(repositoryUrl, location))
return store.dispatch(createSnaps([ repository ], location))
.then(() => {
expect(location).toExcludeKey('href');
expect(store.getActions()).toHaveActionOfType(
Expand Down