Skip to content
This repository has been archived by the owner on Sep 12, 2022. It is now read-only.

Support viewing older requests #751

Merged
merged 7 commits into from
Feb 20, 2018
Merged

Support viewing older requests #751

merged 7 commits into from
Feb 20, 2018

Commits on Jan 24, 2018

  1. Remove unused onFetchModel

    @cdosborn
    cdosborn committed Jan 24, 2018
    Configuration menu
    Copy the full SHA
    03a3ade View commit details
    Browse the repository at this point in the history

  2. Allow the store to fetch/get a single model 

    For collections of models we support `fetchModels` and `getAll`, for individual
models we have `fetchModel` and `get`. However the latter methods don't work
like you might hope.

Problem: 1
If you call `get(modelId)` that checks if the model exists and then fetches the
first page of the models from the server. If `modelId` isn't in the first page,
then `get` won't work.

Problem: 2
`fetchModel` actually fetches the modelId from the server, so theres no concern
about pagination, but it sticks its result `this.models`. If this.models
doesn't exist, then you'll get a runtime error. So `fetchModel` only works if
you implicitly have called `getAll` or `fetchModels` first. Not nice.

Solution to: 1
I created a `getModel` analog to `get` which actually fetches the model. I
couldn't replace `get` because the current behavior is used in many places.

Solution to: 2
I fixed `fetchedModels` in a backwards compatible way to prevent the runtime
error. When it fetches a model it adds it to the model cache (like it used to)
and it adds it to `this.fetchedModels`, which is a cache for model instances.
This fixes the implicit need to call store.getAll() as mentioned.
    @cdosborn
    cdosborn committed Jan 24, 2018
    Configuration menu
    Copy the full SHA
    62e4ada View commit details
    Browse the repository at this point in the history

  3. All resource requests are listed (not just pending ones) 

    There are a few caveats, I'll comment on the diff of this commit

When a resource request gets updated to approved/closed/denied, we no longer
remove it from the store, we show all requests of all status types now

    diff --git a/troposphere/static/js/actions/AdminResourceRequestActions.js b/troposphere/static/js/actions/AdminResourceRequestActions.js
    index 10dc13abd..f31263844 100644
    --- a/troposphere/static/js/actions/AdminResourceRequestActions.js
    +++ b/troposphere/static/js/actions/AdminResourceRequestActions.js
    @@ -14,9 +14,6 @@ export default {
                     Utils.dispatch(AdminResourceConstants.UPDATE, {
                         model: request
                     });
    -                Utils.dispatch(AdminResourceConstants.REMOVE, {
    -                    model: request
    -                });
                 });
             return promise;
         }

We want to make permalink work for admin resource requests. Normally we fetch
all resource request (AdminResourceRequestStore.getAll()) and lookup the
request id in the url from those requests. However, getAll only returns the
first page, sometimes the id is not in the url, so we sometimes fetch the
specific request.

    diff --git a/troposphere/static/js/components/admin/ResourceMaster.jsx b/troposphere/static/js/components/admin/ResourceMaster.jsx
    index d594c2643..111c9c2f7 100644
    @@ -96,57 +94,48 @@ const ResourceMaster = React.createClass({
    +        // Lookup selectedRequest by requestId, or make a separate request for
    +        // the model specifically
    +        let selectedRequest =
    +            (requests.get(requestId) || stores.AdminResourceRequestStore.getModel(requestId));

We sort resource requests first by status then by creation date. The api
returns results sorted by creation date, that's implicit, not great.

    diff --git a/troposphere/static/js/components/admin/ResourceRequest/ResourceRequestNav.jsx b/troposphere/static/js/components/admin/ResourceRequest/ResourceRequestNav.jsx
    index 6eb23462b..72992a2f0 100644
    --- a/troposphere/static/js/components/admin/ResourceRequest/ResourceRequestNav.jsx
    +++ b/troposphere/static/js/components/admin/ResourceRequest/ResourceRequestNav.jsx
    @@ -1,6 +1,13 @@
     import React from "react";
     import Backbone from "backbone";

    +function sortByStatus(request) {
    +    if (request.get('status').name == "pending") {
    +        return 1;
    +    }
    +    return 2;
    +}

    ...

    @@ -51,7 +69,12 @@ export default React.createClass({

             return (
             <ul style={list}>
    -            { requests.map(this.renderListItem) }
    +            {
    +                // Requests are assumed to be sorted by start date, we then
    +                // further sort to render the pending requests first (sortBy
    +                // is stable)
    +                requests.sortBy(sortByStatus).map(this.renderListItem)
    +            }
    @cdosborn
    cdosborn committed Jan 24, 2018
    Configuration menu
    Copy the full SHA
    e7ce465 View commit details
    Browse the repository at this point in the history

  4. Add status of request in request view

    @cdosborn
    cdosborn committed Jan 24, 2018
    Configuration menu
    Copy the full SHA
    06ea1db View commit details
    Browse the repository at this point in the history

  5. Add created section to resource request view

    @cdosborn
    cdosborn committed Jan 24, 2018
    Configuration menu
    Copy the full SHA
    45477b0 View commit details
    Browse the repository at this point in the history

  6. Render an overlay over the update instructions for a resource request 

    When an admin views a resource request that has already been
approved/closed/denied, require interaction to prevent accidental changing of
resources.

Not a huge fan of this approach, think we can redesign this view to be
simpler, more intuitive, but the primary goal is to make the view more usable.
    @cdosborn
    cdosborn committed Jan 24, 2018
    Configuration menu
    Copy the full SHA
    44fcf81 View commit details
    Browse the repository at this point in the history

  7. Use props.router to navigate 

    This fixes the not so nice experience of approving/denying reloading the app.
    @cdosborn
    cdosborn committed Jan 24, 2018
    Configuration menu
    Copy the full SHA
    3f8fed6 View commit details
    Browse the repository at this point in the history