joeflow icon indicating copy to clipboard operation
joeflow copied to clipboard

Usage in App is not working because of namespace issues

Open peterbaumert opened this issue 1 year ago • 7 comments

https://github.com/codingjoe/joeflow/blob/8d58695eeee2502d731fb5dde2491dba63e28c09/joeflow/models.py#L177C29-L177C29

If I use joeflow in an app in my project, the method to get the url namespace returns a wrong resulst.

it should be something like

return f"{cls._meta.app_label}:{cls.__name__.lower()}"

maybe somehow conditionally check if cls._meta.app_label exists

additionally one needs to specify the urls as follows:

path(
        "whateveriwant/",
        include(workflows.VeryCoolWorkflow.urls(), namespace="verycoolworkflow"),
    ),

peterbaumert avatar Aug 09 '23 09:08 peterbaumert

Hi @peterbaumert,

Thank you for reaching out. However, I am not 100% sure I fully understand the issue you are describing. What kind of exception are you seeing? Maybe include the actual exception message and stack trace. A bit of real life code, might also go a long way here.

Anyhow, based on what I see, providing your own namespace, should not be needed. In fact, it may cause the error you are experiencing. The Workflow.url class method returns a tuple including a namespace. If you want to alter the namespace of your workflow, you will need to override the get_url_namespace class method.

I hope this helps!

Cheers, Joe

codingjoe avatar Aug 22 '23 16:08 codingjoe

So if I just add the urls (where I also have app_name = "services") like

path("fileshare/orders/", include(workflows.FileshareOrderWorkflow.urls())),

i get

Traceback (most recent call last):
  File "/PATH/.venv/lib/python3.11/site-packages/django/urls/base.py", line 71, in reverse
    extra, resolver = resolver.namespace_dict[ns]
                      ~~~~~~~~~~~~~~~~~~~~~~~^^^^
KeyError: 'fileshareorderworkflow'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/PATH/.venv/lib/python3.11/site-packages/django/urls/base.py", line 82, in reverse
    raise NoReverseMatch("%s is not a registered namespace" % key)
django.urls.exceptions.NoReverseMatch: 'fileshareorderworkflow' is not a registered namespace

peterbaumert avatar Aug 28 '23 11:08 peterbaumert

So i checked a littel more, it does add the urls in the right app namespace, here "services:fileshareorderworkflow:details" but the get_absolute_url() functions searches for "fileshareorderworkflow:details" and is skipping the app it runs in.

peterbaumert avatar Sep 04 '23 12:09 peterbaumert

Yes, that seems to be an issue. However, we don't have access to the resolver in the get_absolute_url, or rather: We don't know the full name space.

I see two options:

  1. You simply override the method in your implementation.
  2. We add the _meta.app_label variable and pass that as the current_app value. It's not going to be bulletproof, but in those edge cases, solution 1, is probably the best approach.

What do you think? Do you still care to provide a patch?

codingjoe avatar Sep 04 '23 17:09 codingjoe

i will have a look those days yes.

  1. i tried, somehow without success but had to drop the test due other things ^^ as i said, will look into it.

peterbaumert avatar Sep 04 '23 19:09 peterbaumert

So I took some time to look into it.

One issue is that https://github.com/codingjoe/joeflow/blob/0906fb5c0b24e7e11dd675d1f01a68533acfbb62/joeflow/models.py#L165 has to return only the "lower class name" and in the get_absolute_url() functions it needs to return the "app prefixed" string.

So I guess this needs to either be split in two functions or in that line above changed to return urls, cls.__name__.lower().

What do you think?

peterbaumert avatar Sep 05 '23 15:09 peterbaumert

Wait, I don't think I get your point. Isn't this identical, since that's how get_url_namespace is implemented: https://github.com/codingjoe/joeflow/blob/0906fb5c0b24e7e11dd675d1f01a68533acfbb62/joeflow/models.py#L178-L180

codingjoe avatar Nov 24 '23 10:11 codingjoe