Move reverseUrl into reverse() scope

mondala commented 9 years ago

Suggestion: In order to use a URL whose top level is not / but is /myprojectname, django-angular will try to send a URL that starts with /angular/reverse/.... This will break the app with a 404 error. This should actually be built as myProjectName+reverseUrl.

I fixed this in my own project:

[Simple] code changes as noted below in diffs. Essentially, move reverseUrl into the djangoUrl service as a property.
Ensure that djangular.urls are included in the URLconf, such as: url(r'^angular/', include('djangular.urls', namespace='djangular')),
Inject the djangoUrl service into a directive that can initialize its reverseUrl property. For a single page I chain the following directive to the .module().

HTML:

<body init-djng url-root="/myProjectName">

Angular app.js:

      .directive('initDjng',function(){
          return {
              controller: ['djangoUrl','$scope',function(djangoUrl,$scope){
                      djangoUrl.reverseUrl = $scope.urlRoot + djangoUrl.reverseUrl;
              }],
              restrict: 'A',
              scope: { urlRoot : '@urlRoot' }
          };
      });

EDIT: Deleted a step here that was wrong and stated to add something to "the app." It is taken care of in step 1.

With this in place, I can set <... init-djing url-root='....'> to myProjectName so that it's routed to the right URL.

jrief commented 9 years ago

The URL to reverse URLs on the client is hard-coded into this middleware: https://github.com/jrief/django-angular/blob/master/djangular/middleware.py

It has to be hard-coded, because it is evaluated even before Django's URL router "sees" that URL.

I once tried to use urlpatterns with a Redirection, but they were unable to handle POST requests.

jkosir commented 9 years ago

Well yes, /angular/reverse/ url won't work if django app isn't top level. Shouldn't be the case often, but there's no hurt in supporting it.

But instead of doing it your way, I think it would be nicer to set it in config stage. Thoughts?

Btw, step 2) isn't required anymore.

mondala commented 9 years ago

The URL to reverse URLs on the client is hard-coded into this middleware: https://github.com/jrief/django-angular/blob/master/djangular/middleware.py

It has to be hard-coded, because it is evaluated even before Django's URL router "sees" that URL.

Hi Jrief, all I want to do is "correct" the client side because it has no way of knowing the correct top-level path to eventually build the URL, since it looks like django-angular is assuming a top level directory of '' when it could be something else. So, happily it seems there is no need to modify any Django code. The middleware will still process the path correctly because all of the top-level URL is removed from URL before attemping to match to routes.

Maybe I am missing something here. Is the lack of POST redirection a deal-killer for adding the top-level URL? I always thought that redirected POSTs were not possible since they get resent to GETs.

mondala commented 9 years ago

Well yes, /angular/reverse/ url won't work if django app isn't top level. Shouldn't be the case often, but there's no hurt in supporting it.

But instead of doing it your way, I think it would be nicer to set it in config stage. Thoughts?

Sounds good. I actually don't know what the accepted pattern is for Angular service initializations. Less code and no need to create a new .directive() is likely better.

Btw, step 2) isn't required anymore.

I wasn't able to get it to work without including the djangular.urls. :(

jrief commented 9 years ago

@bafflermeal did you add

MIDDLEWARE_CLASSES = (
    'djangular.middleware.DjangularUrlMiddleware',
    ...
)

to your settings.py?

mondala commented 9 years ago

Yep.

MIDDLEWARE_CLASSES = (
    'djangular.middleware.DjangularUrlMiddleware',
    'django.contrib.sessions.middleware.SessionMiddleware',
    'django.middleware.common.CommonMiddleware',

mondala commented 9 years ago

jrief, I apologize. I made a mistake in describing the last step in the first post above that might be the cause of some confusion. I've changed it since it made it read as if I edit in the urlRoot value in to either the JS or the Django app code, when in reality the only constant being passed is in the url-root attribute in init-djng in the .html file.

jkosir commented 9 years ago

Commit d8028cf0093aebf65d5c0ff5473240c97c8f7526

reverse_url can now be set like so:

myApp.config(function(djangoUrlProvider) {
  djangoUrlProvider.setReverseUrl('custom.com/angular/reverse/');
});

The request path on django server has to be angular/reverse/ so middleware can detect it.

jkosir commented 9 years ago

@jrief Any plans on releasing recent updates? I see your update to url middleware also hasn't been released yet?

jrief commented 9 years ago

@jkosir thanks for reminding. I just updated docs/changelog.rst, could you please add the remaining issues, then I will release 0.7.15

jkosir commented 9 years ago

Done. Also updated docs, added a description of setting custom reverse url.

jrief / django-angular

Move reverseUrl into reverse() scope #169