Skip to content

feat(api): Handle project redirect in API client #8799

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jul 26, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 26 additions & 2 deletions src/sentry/api/bases/project.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
from __future__ import absolute_import

from rest_framework.response import Response

from sentry import roles
from sentry.api.base import Endpoint
from sentry.api.exceptions import ResourceDoesNotExist, ResourceMoved
from sentry.api.exceptions import ResourceDoesNotExist, ProjectMoved
from sentry.app import raven
from sentry.auth.superuser import is_active_superuser
from sentry.models import OrganizationMember, Project, ProjectStatus, ProjectRedirect
Expand Down Expand Up @@ -121,7 +123,19 @@ def convert_args(self, request, organization_slug, project_slug, *args, **kwargs
redirect_slug=project_slug
)

raise ResourceMoved(detail={'slug': redirect.project.slug})
# get full path so that we keep query strings
requested_url = request.get_full_path()
new_url = requested_url.replace(
'projects/%s/%s/' %
(organization_slug, project_slug), 'projects/%s/%s/' %
(organization_slug, redirect.project.slug))

# Resource was moved/renamed if the requested url is different than the new url
if requested_url != new_url:
raise ProjectMoved(new_url, redirect.project.slug)

# otherwise project doesn't exist
raise ResourceDoesNotExist
except ProjectRedirect.DoesNotExist:
raise ResourceDoesNotExist

Expand All @@ -139,3 +153,13 @@ def convert_args(self, request, organization_slug, project_slug, *args, **kwargs

kwargs['project'] = project
return (args, kwargs)

def handle_exception(self, request, exc):
if isinstance(exc, ProjectMoved):
response = Response({
'slug': exc.detail['extra']['slug'],
'detail': exc.detail
}, status=exc.status_code)
response['Location'] = exc.detail['extra']['url']
return response
return super(ProjectEndpoint, self).handle_exception(request, exc)
17 changes: 13 additions & 4 deletions src/sentry/api/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,6 @@ class ResourceDoesNotExist(APIException):
status_code = status.HTTP_404_NOT_FOUND


class ResourceMoved(APIException):
status_code = status.HTTP_302_FOUND


class SentryAPIException(APIException):
code = ''
message = ''
Expand All @@ -28,6 +24,19 @@ def __init__(self, code=None, message=None, detail=None, **kwargs):
super(SentryAPIException, self).__init__(detail=detail)


class ProjectMoved(SentryAPIException):
status_code = status.HTTP_302_FOUND
# code/message currently don't get used
code = 'resource-moved'
message = 'Resource has been moved'

def __init__(self, new_url, slug):
super(ProjectMoved, self).__init__(
url=new_url,
slug=slug,
)


class SsoRequired(SentryAPIException):
status_code = status.HTTP_401_UNAUTHORIZED
code = 'sso-required'
Expand Down
7 changes: 6 additions & 1 deletion src/sentry/static/sentry/app/__mocks__/api.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,10 @@ class Client {
}

wrapCallback(id, error) {
return (...args) => respond(Client.mockAsync, error, ...args);
return (...args) => {
if (this.hasProjectBeenRenamed(...args)) return;
respond(Client.mockAsync, error, ...args);
};
}

requestPromise(url, options) {
Expand Down Expand Up @@ -125,5 +128,7 @@ Client.prototype.uniqueId = RealClient.Client.prototype.uniqueId;
Client.prototype.bulkUpdate = RealClient.Client.prototype.bulkUpdate;
Client.prototype._chain = RealClient.Client.prototype._chain;
Client.prototype._wrapRequest = RealClient.Client.prototype._wrapRequest;
Client.prototype.hasProjectBeenRenamed =
RealClient.Client.prototype.hasProjectBeenRenamed;

export {Client};
8 changes: 8 additions & 0 deletions src/sentry/static/sentry/app/actionCreators/modal.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -100,3 +100,11 @@ export function openIntegrationDetails(options = {}) {
});
});
}

export function redirectToProject(newProjectSlug) {
import(/* webpackChunkName: "RedirectToProjectModal" */ 'app/components/modals/redirectToProject')
.then(mod => mod.default)
.then(Modal => {
openModal(deps => <Modal {...deps} slug={newProjectSlug} />, {});
});
}
35 changes: 31 additions & 4 deletions src/sentry/static/sentry/app/api.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@ import $ from 'jquery';
import {isUndefined, isNil} from 'lodash';
import idx from 'idx';

import {openSudo} from 'app/actionCreators/modal';
import {
PROJECT_MOVED,
SUDO_REQUIRED,
SUPERUSER_REQUIRED,
} from 'app/constants/apiErrorCodes';
import {openSudo, redirectToProject} from 'app/actionCreators/modal';
import GroupActions from 'app/actions/groupActions';

export class Request {
Expand Down Expand Up @@ -53,6 +58,23 @@ export class Client {
return s4() + s4() + '-' + s4() + '-' + s4() + '-' + s4() + '-' + s4() + s4() + s4();
}

/**
* Check if the API response says project has been renamed.
* If so, redirect user to new project slug
*/
hasProjectBeenRenamed(response) {
let code = response && idx(response, _ => _.responseJSON.detail.code);

// XXX(billy): This actually will never happen because we can't intercept the 302
// jQuery ajax will follow the redirect by default...
if (code !== PROJECT_MOVED) return false;

let slug = response && idx(response, _ => _.responseJSON.detail.extra.slug);

redirectToProject(slug);
return true;
}

wrapCallback(id, func, cleanup) {
/*eslint consistent-return:0*/
if (isUndefined(func)) {
Expand All @@ -65,6 +87,11 @@ export class Client {
delete this.activeRequests[id];
}
if (req && req.alive) {
// Check if API response is a 302 -- means project slug was renamed and user
// needs to be redirected
if (this.hasProjectBeenRenamed(...args)) return;

// Call success callback
return func.apply(req, args);
}
};
Expand All @@ -81,12 +108,12 @@ export class Client {

handleRequestError({id, path, requestOptions}, response, ...responseArgs) {
let code = response && idx(response, _ => _.responseJSON.detail.code);
let isSudoRequired = code === 'sudo-required' || code === 'superuser-required';
let isSudoRequired = code === SUDO_REQUIRED || code === SUPERUSER_REQUIRED;

if (isSudoRequired) {
openSudo({
superuser: code === 'superuser-required',
sudo: code === 'sudo-required',
superuser: code === SUPERUSER_REQUIRED,
sudo: code === SUDO_REQUIRED,
retryRequest: () => {
return this.requestPromise(path, requestOptions)
.then((...args) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
import {Flex} from 'grid-emotion';
import {withRouter} from 'react-router';
import PropTypes from 'prop-types';
import React from 'react';
import styled from 'react-emotion';

import {t, tct} from 'app/locale';
import Button from 'app/components/buttons/button';
import Text from 'app/components/text';
import recreateRoute from 'app/utils/recreateRoute';

class RedirectToProjectModal extends React.Component {
static propTypes = {
/**
* New slug to redirect to
*/
slug: PropTypes.string.isRequired,

Header: PropTypes.oneOfType([PropTypes.func, PropTypes.node]).isRequired,
Body: PropTypes.oneOfType([PropTypes.func, PropTypes.node]).isRequired,
};

constructor(props) {
super(props);

this.state = {
timer: 5,
};
}

componentDidMount() {
setInterval(() => {
if (this.state.timer <= 1) {
window.location.assign(this.getNewPath());
return;
}

this.setState(state => ({
timer: state.timer - 1,
}));
}, 1000);
}

getNewPath() {
let {params, slug} = this.props;

return recreateRoute('', {
...this.props,
params: {
...params,
projectId: slug,
},
});
}

render() {
let {slug, Header, Body} = this.props;
return (
<React.Fragment>
<Header>{t('Redirecting to New Project...')}</Header>

<Body>
<div>
<Text>
<p>{t('The project slug has been changed.')}</p>

<p>
{tct(
'You will be redirected to the new project [project] in [timer] seconds...',
{
project: <strong>{slug}</strong>,
timer: `${this.state.timer}`,
}
)}
</p>
<ButtonWrapper>
<Button priority="primary" href={this.getNewPath()}>
{t('Continue to %s', slug)}
</Button>
</ButtonWrapper>
</Text>
</div>
</Body>
</React.Fragment>
);
}
}

export default withRouter(RedirectToProjectModal);
export {RedirectToProjectModal};

const ButtonWrapper = styled(Flex)`
justify-content: flex-end;
`;
3 changes: 3 additions & 0 deletions src/sentry/static/sentry/app/constants/apiErrorCodes.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export const SUDO_REQUIRED = 'sudo-required';
export const SUPERUSER_REQUIRED = 'superuser-required';
export const PROJECT_MOVED = 'project-moved';
27 changes: 5 additions & 22 deletions src/sentry/static/sentry/app/views/projects/projectContext.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import MemberListStore from 'app/stores/memberListStore';
import MissingProjectMembership from 'app/components/missingProjectMembership';
import OrganizationState from 'app/mixins/organizationState';
import ProjectsStore from 'app/stores/projectsStore';
import recreateRoute from 'app/utils/recreateRoute';
import SentryTypes from 'app/sentryTypes';
import withProjects from 'app/utils/withProjects';

Expand Down Expand Up @@ -44,7 +43,6 @@ const ProjectContext = createReactClass({
projectId: PropTypes.string,
orgId: PropTypes.string,
location: PropTypes.object,
router: PropTypes.object,
},

childContextTypes: {
Expand Down Expand Up @@ -197,31 +195,16 @@ const ProjectContext = createReactClass({
errorType: ERROR_TYPES.MISSING_MEMBERSHIP,
});
} else {
// The project may have been renamed, attempt to lookup the project, if
// we 302 we will receive the moved project slug and can update update
// our route accordingly.
const lookupHandler = resp => {
const {status, responseJSON} = resp;

if (status !== 302 || !responseJSON || !responseJSON.detail) {
// The request is a 404 or other error
this.api.request(`/projects/${orgId}/${projectId}/`, {
error: () => {
this.setState({
loading: false,
error: true,
errorType: ERROR_TYPES.PROJECT_NOT_FOUND,
});
return;
}

this.props.router.replace(
recreateRoute('', {
...this.props,
params: {...this.props.params, projectId: responseJSON.detail.slug},
})
);
};

// The request ill 404 or 302
this.api.request(`/projects/${orgId}/${projectId}/`, {error: lookupHandler});
},
});
}
},

Expand Down
19 changes: 19 additions & 0 deletions tests/js/spec/api.spec.jsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import $ from 'jquery';

import {Client, Request, paramsToQueryArgs} from 'app/api';
import GroupActions from 'app/actions/groupActions';
import {PROJECT_MOVED} from 'app/constants/apiErrorCodes';

jest.unmock('app/api');

Expand Down Expand Up @@ -92,6 +94,23 @@ describe('api', function() {
});
});

it('does not call success callback if 302 was returned because of a project slug change', function() {
let successCb = jest.fn();
api.activeRequests = {id: {alive: true}};
api.wrapCallback('id', successCb)({
responseJSON: {
detail: {
code: PROJECT_MOVED,
message: '...',
extra: {
slug: 'new-slug',
},
},
},
});
expect(successCb).not.toHaveBeenCalled();
});

it('handles error callback', function() {
sandbox.stub(api, 'wrapCallback', (id, func) => func);
let errorCb = jest.fn();
Expand Down
Loading