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

Usage in App is not working because of namespace issues #89

Open
peterbaumert opened this issue Aug 9, 2023 · 7 comments
Open

Usage in App is not working because of namespace issues #89

peterbaumert opened this issue Aug 9, 2023 · 7 comments
Assignees
Labels
bug Something isn't working question Further information is requested

Comments

@peterbaumert
Copy link
Contributor

peterbaumert commented Aug 9, 2023

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"),
    ),
@codingjoe
Copy link
Owner

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 codingjoe self-assigned this Aug 22, 2023
@codingjoe codingjoe added the question Further information is requested label Aug 22, 2023
@peterbaumert
Copy link
Contributor Author

peterbaumert commented Aug 28, 2023

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
Copy link
Contributor Author

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.

@codingjoe codingjoe reopened this Sep 4, 2023
@codingjoe
Copy link
Owner

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 codingjoe added the bug Something isn't working label Sep 4, 2023
@peterbaumert
Copy link
Contributor Author

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
Copy link
Contributor Author

So I took some time to look into it.

One issue is that

return urls, cls.get_url_namespace()
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?

@codingjoe
Copy link
Owner

Wait, I don't think I get your point. Isn't this identical, since that's how get_url_namespace is implemented:

joeflow/joeflow/models.py

Lines 178 to 180 in 0906fb5

@classmethod
def get_url_namespace(cls):
return cls.__name__.lower()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants