Skip to content
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

Retrieving a state by name #265

Closed
DylanArnold opened this issue Jul 16, 2013 · 29 comments
Closed

Retrieving a state by name #265

DylanArnold opened this issue Jul 16, 2013 · 29 comments
Labels
Milestone

Comments

@DylanArnold
Copy link

Hello,

I'm looking at adding a permissions system to my app and I am looking at attaching some data to a state which defines the permissions a user must have in order to access a state.

Example definition:

$stateProvider.state('admin', {
    url: '/admin/:id',
    templateUrl: '/admin.html',
    controller: 'AdminCtrl',
    data: {
        auth: ['Staff', 'Admin'] // Could be a callback which is passed the state params
    }
});

This data could be a set of roles which a user is required to have before they can access the state, or it could potentially contain a callback which would ideally have access to the parameters passed to a state to determine access to a state on a more fine grained (parameter determined) level.

This seems like it will work fine to restrict access to certain states. I can write a $stateChangeStart authorization plugin which will check the next state, and redirect the user to a login or permission denied page if they don't have access, based on the data I defined on my state.

So far this is all possible. The problem I am trying to figure out is how to only display certain elements on a page such as navigation links based on these same permission tests.

I am thinking of writing a directive that would retrieve a state and run the same permission tests as above, showing or hiding the target element. At first, for the case where I want to show or hide navigation elements, I wanted to retrieve a state given a url, but haven't found a way to do this yet. Then I saw the ui-sref directive, it would make just as much sense to create a directive similar to this, or modify this directive but also retrieve the state by name, run my authorization logic (passing any parameters through), and show or hide the element depending on result.

How can I retrieve a state by name? As far as I can tell, there are no accessors for this. Is there another way to accomplish this?

Thanks for your time.

TLDR
How do I retrieve a state by name?

@timkindberg
Copy link
Contributor

There's in internal method in the $state provider, findState(), but there is no way to find a state externally. You'd have to alter the code and do a custom build, or we'd have to add this feature.

@ksperling would this be a dangerous feature to add, would it promote more anti-patterns? I'm not sure why but it sounds like a bad thing to allow. Then people could alter states on the fly, and who knows what would happen...

@DylanArnold
Copy link
Author

My first thought in reponse to that was, hang on, isn't the state exposed in $stateChangeStart()?

After a little bit more inspection it appears as though in registerState() the original object is wrapped in the self property of an internal object to manage some private data https://github.com/angular-ui/ui-router/blob/master/src/state.js#L22

I'm assuming for the public interface, there shouldn't be a need to access the internal object, but if the stored self object is considered public then retrieving a state by name should return that. As an end consumer I shouldn't ever need to know about the internal representation or be exposed to it.
I may be wrong but from what I can tell you can think of a state in terms of the public interface, the object passed to $stateProvider.state(), and the private state, retrieved by findState() which I should never need to know about.

It appears this is the case for $stateChangeStart(). As you can see here https://github.com/angular-ui/ui-router/blob/master/src/state.js#L195 it returns the .self object.

@braco
Copy link

braco commented Jul 23, 2013

So how can you check if a state exists? Even an array of available state names would be helpful.

It would be handy for something like this:

$rootScope.$on('$stateChangeStart', function(evt, toState, toParams, fromState, fromParams) {

var testFor = toState.name + "loggedin";

if (user.loggedIn === true && STATE_EXISTS(testFor)) { 
//  transitionTo(testFor)
}

})

Allowing special case states to be used (that may or may not exist)

@eahutchins
Copy link

This seems like a basic feature required for lazy loading states, why was the issue closed? Introspecting the current state graph would be useful for writing general purpose debugging code as well.

@laurelnaiad
Copy link

You can't lazy load states in ui-router. If you're a good coder and don't mind getting your hands dirty, you're welcome to pick up where I've temporarily left off with angular-detour. Drop me a line in an issue there if you want to use it and can't figure out how.

Keep in mind that you need an active server to lazy load states from the server based on URLs or state names.

If you just want to modify/add/delete state definitions on the fly at runtime in angular, that's very straightforward in detour.

Its biggest drawback at the moment is that it's about 1 month and a half behind the commits to ui-rotuer.

@laurelnaiad
Copy link

For those who, as the OP wanted, just want to find out about your states, define them in a hash object (with any extra bells and whistles that you will want at runtime) outside of calls to .state... then just iterate over your object to call .state to define the routing and hang onto your custom state definitions when you need to know something about them.

@eahutchins
Copy link

I've implemented the lazy-loading part using our own back-end secret sauce, and I've been cribbing from the work you and others have done for how what I've loaded gets patched into ui-router/angular. I can obviously build my own cache of which states have already been lazy-loaded, but it seems like a generally useful feature for ui-router to support (and some of the ideas for a debugging tools that I'm thinking about would benefit from going to the canonical source of truth about what the currently loaded state graph looks like).

@laurelnaiad
Copy link

Sounds like you just want to patch ui-router if all you need is to make findState public. If you do that and exposes the .state function in the service, you've got the underpinnings of a busy/poor man's lazy load-able version.

@DylanArnold
Copy link
Author

So far I haven't seen any arguments against this feature, it seems like a no brainer to me.

Yeah I ended up writing my own service to wrap $stateProvider.state and hold references to each state. It's a bit of a hack but it works and seems less hackish than patching the source.

I was thinking about submitting a pull request but I'd prefer to know if it's just going to get rejected.

@DylanArnold
Copy link
Author

By the way it isn't findState that needs to be exposed, it is the reference to the object stored in self which is considered public anyway.

@laurelnaiad
Copy link

Search closed issues you may find some arguments against. I never wanted to "take a detour" but lazy and runtime changes werent considered a priority and/or too complicated.

@DylanArnold
Copy link
Author

I feel like you haven't read my issue very thoroughly. I at least wasn't asking for runtime changes. This is a very simple issue, and the object I mention is already exposed in $stateChangeStart or you can keep a reference by yourself, so I don't yet see how this could be a problem.

@laurelnaiad
Copy link

I don't mean to dissuade you. I thought you were talking about lazy
loading states, which I interpreted to mean changes to states at run-time.

On Thu, Jul 25, 2013 at 3:29 PM, DylanArnold notifications@github.comwrote:

I feel like you haven't read my issue very thoroughly. I at least wasn't
asking for runtime changes. This is a very simple issue, and the object I
mention is already exposed in $stateChangeStart or you can keep a
reference by yourself, so I don't yet see how this could be a problem.


Reply to this email directly or view it on GitHubhttps://github.com//issues/265#issuecomment-21589481
.

@timkindberg
Copy link
Contributor

The reason it was closed—certainly not with an iron fist and the issue could be reopened—is because accessing the state config objects at run time may give the incorrect impression that those config objects can be altered at any time. We have not yet fully determined if we want to allow runtime editing of state properties, we have decided "no" for now, certainly until we've reached a stable code base with the current feature set.

If there was some way to ensure that the state map was read only and devs knew it, I would be open to reopening this feat request.

@nateabele what do you think, am I wrong in my assumption?

@timkindberg
Copy link
Contributor

I think if we just say that the returned map of states is read only this could be ok.

@DylanArnold
Copy link
Author

I can see where you are coming from if modifying the config object can indeed change runtime behaviour. You'd have to be pretty careful in that case.

In a way though it seems that this is already a risk if indeed the original config object is the same one used internally and passed around and exposed to the user in events.

It also seems to be the case that providing an accessor for this object does tell a developer to do as you like with it whereas it isn't so explicit that this is the case when it's available during events.

If modifying the config object can break stuff, maybe the internals should operate on a copy of it? - However it seems like what you may be saying is that this is already the case and providing an accessor would make developers incorrectly think that it is possible to change runtime behaviour.

I can see why the issue was closed and it obviously is something that needs some thought. My other idea if this was the case was that a middle ground may be to provide an accessor for the user provided data object, or even methods to inspect parts of a state in a readonly manner (but that seems cumbersome).

@laurelnaiad
Copy link

The config object being passed around to the user isn't the one operating
at run-time. The internals immediately make a copy of it and operate on
that copy.

@timkindberg
Copy link
Contributor

@DylanArnold, I'm in complete agreement. We need to see what @nateabele and @ksperling think.

@nateabele
Copy link
Contributor

As @DylanArnold alludes to, the current internals exposure is already sort of an issue. I think @ksperling was just nervous about doing anything hasty before committing to an API for forever.

@DylanArnold
Copy link
Author

I suppose it is, and it isn't an issue, at least with my current thinking on it.

I'm not saying the following is how it is currently, or how it should be. It's just a way to think about it.

If you consider the object you pass to $stateProvider.state() to be a configuration object, it is just that, configuration. It's the initial call to $stateProvider.state() which is doing the configuring, after which your really done with your configuration. After that initial call why should altering any properties on that object alter any internals, they shouldn't. This has already been considered since the internals operate on a copy.

As soon as I consider the objects passed around in events like $stateChangeStart to be the state configuration the problem feels like it goes away. Sure they aren't immutable (since this is javascript), but should be considered such. In general when I deal with configuration I don't expect changes to it to affect my runtime, without reconfiguring (which could potentially be possible by calling some method).

I guess if this makes some sort of sense conceptually then the method I'd be asking for is really getStateConfig(), however since I am already really dealing with configuration everywhere and it would be bad to randomly add a new concept it could really just be short handed to getState().

Anyway I think what I'm trying to say is that if I think about it in this way it doesn't feel like the way this object is exposed currently is a problem. If anything at all, it mostly feels like a naming / education / definitions problem.

@jeme
Copy link
Contributor

jeme commented Jul 26, 2013

I don't see any harm in this, there is the potential of "misunderstandings" around what you can use the object returned for, but that is where documentation or proper naming comes in to the picture.

If we really wanted to go about it and be difficult, then it might just be named "getStateCopy" indicating that it is fact not the actual configuration object, but rather a copy of it, which should state clearly "changing this doesn't have any effect for the StateProvider"...

And it doesn't need to be getStateConfigurationCopy, because the object you pass in though the .state method which we wrap in the .self property, is what externally is known to be the state, the other part is more an "implementation" detail needed to make sure we don't mangle any existing properties on the state, but that isn't externally known.

Anyways, If it provides a feature that some is asking for, i see this as an small and risk-less addition.

Should be an easy thing to add, and I'dd be happy to do so if we can come to some agreement.

@timkindberg timkindberg reopened this Jul 26, 2013
@timkindberg
Copy link
Contributor

I'm fine with this now. I kind of like $state.find(). Or maybe this should be a method on $stateProvider?

@eahutchins
Copy link

Reading through #146 (arg, meant #126) it seems that dynamic state modifications aren't on the roadmap... is this the consensus? Dynamic states are exactly what I've got a use case for (think CMS where the contents are web applications).

FWIW I think $state.exists(...) as a test of whether a state exists is most intuitive.

@laurelnaiad
Copy link

Funny you should mention that use case: #126

@ncuillery
Copy link
Contributor

I tried to generate a breadcrumb from the current state : Each state have a parent, a relative url and a custom attribute "data" for label. So I had all I need... expect a "findStateByName" method for intermediate steps of the breadcrumb. It made me to do a dirty work-around by keeping a reference of the declared states :-(

@timkindberg

I'm fine with this now. I kind of like $state.find(). Or maybe this should be a method on $stateProvider?

The first one for my use case : $state.find() is easy to reach from a controller.

@nateabele
Copy link
Contributor

Closing because $state.get() is a thing now.

@christopherthielen christopherthielen modified the milestones: 0.2.8, 1.5.0 Nov 16, 2014
@ghelton
Copy link

ghelton commented Dec 20, 2014

Is there a $stateProvider accessible way of retrieving the currently loaded states? The site I'm working on has some 'hardcoded' states, and some states which are registered in a provider during the app's config state. The app registers, then all standard views are loaded, then all cms defined views load.

I'd like to not have to specify in my cms if a view is a hardcoded one, or a dynamically generated one. I need the overlapping definitions, the server provides data(copy, documents) to be displayed, and the template lays them out.

If I could check if a state is already registered then I could skip the dynamic state registration for that specific route.

My provider code looks like this:

_.forEach(preloaded.byRoute, function(obj, key) {
    var title = obj.name;
    $stateProvider.state(obj.route, {
        url: '/' + obj.route,
        templateUrl: 'app/dynamic-view/dynamic-view.html',
        controller: 'DynamicViewCtrl',
        title: obj.title,
        resolve: {
            document: ['PrismicLoader', function(PrismicLoader) {
                return PrismicLoader.loadRoute(this.self.name);
            }]
        }
    });
});

@IllyaMoskvin
Copy link

IllyaMoskvin commented Jun 28, 2017

I'm running into the same problem. I need to check if a state exists during a provider's config phase, also for dynamic state registration. @Astrism, I know it's been a while, but do you recall how you resolved this issue?

@IllyaMoskvin
Copy link

My naive solution was to create a provider called $stateConfig, which keeps track of state names. This new provider "wraps" calls to $stateProvider, so it's pluggable: I just substituted $stateConfigProvider for wherever I was previously using $stateProvider.

This new provider contains an exists( name ) method, which will return a boolean if a state with a certain name has already been defined. I thought of calling this method get() to match $state.get(), but since I'm only interested in checking if the state exists (not retrieving its full config), exists() seemed more appropriate.

Everything seems to be working so far for my needs, but consider this super-duper untested:

(function () {
    'use strict';

    angular
        .module('app')
        .provider('$stateConfig', Provider);

    // $state.get() allows us to check if a state exists,
    // but there's no equivalent method for $stateProvider.
    // https://github.com/angular-ui/ui-router/issues/265

    // $stateConfigProvider stores the state's name,
    // before deferring calls to $stateProvider.

    // Use $stateConfigProvider.exists() to check if a state exists

    Provider.$inject = ['$stateProvider'];

    function Provider( $stateProvider ) {

        var states = [];

        this.state = state;
        this.decorator = $stateProvider.decorator;

        this.exists = exists;

        this.$get = function() {
            return this;
        };

        return this;

        function state( name, data ) {
            states.push( name );
            $stateProvider.state.apply( this, arguments );
            return this;
        }

        function exists( name ) {
            return states.indexOf( name ) > -1;
        }

    }
    
})();

IllyaMoskvin added a commit to art-institute-of-chicago/data-dashboard that referenced this issue Jun 28, 2017
IllyaMoskvin added a commit to art-institute-of-chicago/data-dashboard that referenced this issue Aug 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests