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

Ms routes app page #295

Merged
merged 8 commits into from
May 31, 2016
Merged

Ms routes app page #295

merged 8 commits into from
May 31, 2016

Conversation

msecret
Copy link
Contributor

@msecret msecret commented May 27, 2016

Built on top of #290, that should be closed first.

This adds a table below the basic info on the app page for routes. To do this, a RouteStore has been created with all the things that go along with that (actions, constants).

Currently work in progress.

This was referenced May 27, 2016
Marco Segreto added 5 commits May 27, 2016 14:29
New actions to get all routes for an app. These were separate methods
so that the appGuid could be passed along. The route data that comes
from this request doesnt' include the appGuid that the routes belong
to, so we pass this guid around so the route store will add it to the
data structure. This method of doing this is consistent with the rest
of the codebase, and ensures only the stores change data, rather then
the API doing it. It also means consistency, as if routes are gotten
in a manner where there will be no app guid, then the store has to
handle that case.
Pretty simple, what happens on fetch and received.
Uses an normal table as I don't really need the functionality of
reactable and I want to get rid of that library anyway.

I do the filtering of the routes so I only get the routes thet bleong
to the current app in the state setter in the route list component. This
is a little different but might be a good solution rather then adding
more methods onto the store.
@coveralls
Copy link

Coverage Status

Coverage remained the same at 72.599% when pulling dc033c7 on ms-routes_app_page into e94c1f5 on staging-alpha.

Simple actions for fetching and receiving a domain by domain
guid.
@coveralls
Copy link

Coverage Status

Coverage remained the same at 72.599% when pulling 5f9ed28 on ms-routes_app_page into e94c1f5 on staging-alpha.

Marco added 2 commits May 27, 2016 15:49
The route store's guid has to be matched with an existing route
domain_guid. If no route with matching domain_guid exists, it should
not do anything.

Rather the add new functionality onto base store, this was done by
constructing a specific object to use the merge function with. This
object is created with a `domain` key that holds the whole domain
object and then a domain id which is just the domain's guid.

The test seems to show that if the matching route doesn't exist then
no new routes are created, which is correct.
domain
});
}
};
Copy link
Contributor

Choose a reason for hiding this comment

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

@msecret what do you think about consolidating these routeActions and domainActions into routeActions?

Here's my thinking: its kind of some CF specific terminology to know the difference between a route and a domain since most of the time you'll actually want both to determine the URL for an application. By having them handled in the same store, but using different action creators and action types it makes it a bit more confusing to hold the difference between a route and a domain in your head.

I think it would make sense to bring all of this into the route*.js files. Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea I see what you mean. I think I separated them because there is a just Routes and just Domains view in the community UI, so figured we'd need to separate them conceptually at some point.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ya, we can wait for the research team to tell us otherwise, but I have a feeling that the division is hard for most users to understand and that we'll actually be using them in concert.

I'm open either way, which direction do you want to go?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ha the one with less work.

@msecret msecret merged commit 131d628 into staging-alpha May 31, 2016
@jeremiak jeremiak deleted the ms-routes_app_page branch June 2, 2016 20:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants