NoReverseMatch when "save_as = True" enabled

[dacodekid]

With an admin model

class MyModelAdmin(DjangoObjectActions, admin.ModelAdmin):
    save_as = True
    change_actions = ('my_action')

I'm getting following error when I click Save as new button (highlighting line # 7).

NoReverseMatch at /admin/myapp/mymodel/1/change/

Error during template rendering

In template /Users/dacodekid/.envs/myapp/lib/python3.5/site-packages/django_object_actions/templates/django_object_actions/change_form.html, error at line 7
Reverse for 'myapp_mymodel_actions' with arguments '()' and keyword arguments '{'tool': 'my_action', 'pk': None}' not found. 2 pattern(s) tried: ['admin/myapp/mymodel/actions/(?P<tool>\\w+)/$', 'admin/myapp/mymodel/(?P<pk>[0-9a-f\\-]+)/actions/(?P<tool>\\w+)/$']
1   {% extends "admin/change_form.html" %}
2   
3   
4   {% block object-tools-items %}
5     {% for tool in objectactions %}
6       <li class="objectaction-item" data-tool-name="{{ tool.name }}">
7         <a href='{% url tools_view_name pk=object_id tool=tool.name %}' title="{{ tool.standard_attrs.title }}"
8            {% for k, v in tool.custom_attrs.items %}
9              {{ k }}="{{ v }}"
10           {% endfor %}
11           class="{{ tool.standard_attrs.class }}">
12        {{ tool.label|capfirst }}
13        </a>
14      </li>
15    {% endfor %}
16    {{ block.super }}
17  {% endblock %}

[AlexRiina]

I'm not positive this is your issue, but I think you need

-    change_actions = ('my_action')
+    change_actions = ('my_action', )

[dacodekid]

No. That's not it. When pasted, I actually removed the extra actions (and mistakenly the trailing comma) for simplicity. I'll try to recreate a simple app from scratch and see if produces the error and keep it posted here. Thx @AlexRiina

[crccheck]

I'm thinking there's some inconsistency between detecting if it's an "add" or "change" form. What version of Django?

[dacodekid]

@AlexRiina I've created a simple app here which also producing the same error.

@crccheck I'm using the dev version - 1.10.dev20160404211427. But after saw your comment, I uninstalled the dev and installed Django 1.9.5 and still it produces the same error.

[dacodekid]

@AlexRiina / @crccheck

Would you consider the below change on get_change_actions method? https://github.com/crccheck/django-object-actions/compare/master...dacodekid:patch-1

    def get_change_actions(self, request, object_id, form_url):
        """
        Override this to customize what actions get to the change view.

        This takes the same parameters as `change_view`.

        For example, to restrict actions to superusers, you could do:

            class ChoiceAdmin(DjangoObjectActions, admin.ModelAdmin):
                def get_change_actions(self, request, **kwargs):
                    if request.user.is_superuser:
                        return super(ChoiceAdmin, self).get_change_actions(
                            request, **kwargs
                        )
                    return []
        """

        # if the request from 'Save as new' button
        if '_saveasnew' in request.POST:
            return []

        return self.change_actions

[AlexRiina]

The issue is basically that "save as new" is an add option that takes place from the change_form template. When there is an error saving as new (in this case a duplicate value on a unique field), the error is rendered on the change_form template, but there is no object_id like we might expect.

Django avoids this crash with the history button by saving the url as a variable which seems to set it to an empty string in an error case, so the history button leads to the relative path "" in a similar "save as new" error.

Unfortunately, django passes the original object_id into the change_view that django-object-actions overwrites, but only sets the object_id to None inside its own changeform_view after https://github.com/django/django/blob/master/django/contrib/admin/options.py#L1403

The similar handling in django's changeform_view makes a good case for the above change. Another few options to continue the discussion:

1. Overwrite render_change_form instead of change_view so we can intercept the context after it has the correctly blank object_id

# django_object_actions/utils.py
class BaseDjangoObjectActions(object):
    ...
    def render_change_form(self, request, context, add, change, obj, form_url)
        context['tools_view_name'] = self.tools_view_name
        if add:
             context['objectactions'] = [
                self._get_tool_dict(action) for action in
                self.get_change_actions(request, obj.pk, form_url)
             ]
        else:
             context['objectactions'] = []
        return super(BaseDjangoObjectActions, self).render_change_form(
            request, context, add, change, obj, form_url)

1. Wrap the urls in {% if change %}

# django_object_actions/templates/django_object_actions/changeform.html
{% block object-tools-items %}
  {% if change %}
    {% for tool in objectactions %}
...

1. Allow the urls to break like the history url breaks.

# django_object_actions/templates/django_object_actions/changeform.html
        {% url tools_view_name pk=original.pk tool=tool.name as tool_url %}
        <a href='{{ tool_url }}' title="{{ tool.standard_attrs.title }}"

[dacodekid]

@crccheck / @AlexRiina , Is there any update on this? I'm currently using my dirty fix, but hoping to see @AlexRiina's proposal be implemented.

[crccheck]

I haven't had any free time so far, hopefully by this weekend. I'm also looking for a regression test, on #61 or on its own is ok too.

update: oops that PR is for an unrelated issue

[crccheck]

I threw save_as = True on an example admin and I couldn't recreate (Django 1.9.5, Django Object Actions 0.8.2)

With save_as = True on the choice admin, I opened an existing choice, modified the "Choice text" then clicked "Save as new" and was sent to the changelist page for choices. I also clicked on "Save and continue editing" and "SAVE"

diff --git a/example_project/polls/admin.py b/example_project/polls/admin.py
index 4f11017..d7bfec7 100644
--- a/example_project/polls/admin.py
+++ b/example_project/polls/admin.py
@@ -14,6 +14,7 @@ from .models import Choice, Poll, Comment

 class ChoiceAdmin(DjangoObjectActions, admin.ModelAdmin):
     list_display = ('poll', 'choice_text', 'votes')
+    save_as = True

     # Actions
     #########