If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

autoland-ui: UI displays error if the series ID is invalid

RESOLVED FIXED

Status

Conduit
General
RESOLVED FIXED
8 months ago
8 months ago

People

(Reporter: davidwalsh, Assigned: davidwalsh)

Tracking

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

8 months ago
We should display an error if we somehow don't receive a series ID from the UI.
Comment hidden (mozreview-request)
(Assignee)

Updated

8 months ago
Assignee: nobody → dwalsh
(Assignee)

Updated

8 months ago
Attachment #8832136 - Attachment is obsolete: true
Attachment #8832136 - Flags: review?(imadueme)
Comment hidden (mozreview-request)

Comment 3

8 months ago
mozreview-review
Comment on attachment 8832162 [details]
autoland-ui: The UI should display an error if no repo or series ID is provided (Bug 1335481).

https://reviewboard.mozilla.org/r/108506/#review109686

::: autoland/ui/src/routes.jsx:8
(Diff revision 1)
>  import { Router, Route, browserHistory } from 'react-router';
>  
>  import App from './components/App';
>  import TestApp from './components/TestApp';
>  import AutolandController from './components/AutolandController';
> +import ErrorPage from './components/ErrorPage';

This page doesn't exist. Did you forget to commit it?

::: autoland/ui/src/routes.jsx:16
(Diff revision 1)
>    render((
>        <Router history={browserHistory}>
>          <Route path="/" component={App}>
>            <Route path="repos/:repo_id/series/:series_id" component={AutolandController} />
>          </Route>
>          <Route path="/test" component={TestApp} />

TestApp's "404" case should have a repo id provided to be consistent. (It will still 404 since it is an invalid repo/series combination)
Attachment #8832162 - Flags: review?(imadueme) → review-

Comment 4

8 months ago
mozreview-review
Comment on attachment 8832162 [details]
autoland-ui: The UI should display an error if no repo or series ID is provided (Bug 1335481).

https://reviewboard.mozilla.org/r/108506/#review109688

::: autoland/ui/src/routes.jsx:8
(Diff revision 1)
>  import { Router, Route, browserHistory } from 'react-router';
>  
>  import App from './components/App';
>  import TestApp from './components/TestApp';
>  import AutolandController from './components/AutolandController';
> +import ErrorPage from './components/ErrorPage';

Can we rename this to InvalidUrl page or something?

Comment 5

8 months ago
mozreview-review-reply
Comment on attachment 8832162 [details]
autoland-ui: The UI should display an error if no repo or series ID is provided (Bug 1335481).

https://reviewboard.mozilla.org/r/108506/#review109688

> Can we rename this to InvalidUrl page or something?

:)
Comment hidden (mozreview-request)
(Assignee)

Comment 7

8 months ago
mozreview-review-reply
Comment on attachment 8832162 [details]
autoland-ui: The UI should display an error if no repo or series ID is provided (Bug 1335481).

https://reviewboard.mozilla.org/r/108506/#review109686

> TestApp's "404" case should have a repo id provided to be consistent. (It will still 404 since it is an invalid repo/series combination)

I'm still not understanding this.  Small code sample to help me understand?

Comment 8

8 months ago
mozreview-review-reply
Comment on attachment 8832162 [details]
autoland-ui: The UI should display an error if no repo or series ID is provided (Bug 1335481).

https://reviewboard.mozilla.org/r/108506/#review109686

> I'm still not understanding this.  Small code sample to help me understand?

It's not really related to this diff, and adding the repo id wouldn't really change any functionality right now. It's just to maintaint the consistency I talked about before between what the test page shows and what would actually be shown if you went to that page using react router.
Comment hidden (mozreview-request)

Comment 10

8 months ago
mozreview-review
Comment on attachment 8832162 [details]
autoland-ui: The UI should display an error if no repo or series ID is provided (Bug 1335481).

https://reviewboard.mozilla.org/r/108506/#review110000

+1
Attachment #8832162 - Flags: review?(imadueme) → review+

Comment 11

8 months ago
mozreview-review
Comment on attachment 8832162 [details]
autoland-ui: The UI should display an error if no repo or series ID is provided (Bug 1335481).

https://reviewboard.mozilla.org/r/108506/#review110006

r+ again?

Comment 12

8 months ago
mozreview-review
Comment on attachment 8832162 [details]
autoland-ui: The UI should display an error if no repo or series ID is provided (Bug 1335481).

https://reviewboard.mozilla.org/r/108506/#review110018

Rubber stamp for landing.
Attachment #8832162 - Flags: review+

Comment 13

8 months ago
Pushed by mcote@mozilla.com:
https://hg.mozilla.org/automation/conduit/rev/38208123899e
autoland-ui: The UI should display an error if no repo or series ID is provided . r=imadueme,mcote
Status: NEW → RESOLVED
Last Resolved: 8 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.