Add ability to configure sharing domain via Django settings - #78
Conversation
| if get_router_cls() == ".".join( | ||
| [DatabaseHostRouter.__module__, DatabaseHostRouter.__qualname__] | ||
| ): |
There was a problem hiding this comment.
What's the reason for the indirection with __module__ and __qualname__?
There was a problem hiding this comment.
This is checking to see if the router class has been configured as "wagtailsharing.routers.db.DatabaseHostRouter". We could hardcode that string here, but if this code is ever moved to another file, that would break. Using __module__ + __qualname__ generates the same string but does it dynamically so it'll work wherever the code lives.
There was a problem hiding this comment.
Aha! Yeah I suppose that makes sense. It would still require changes in __init__.py and the readme, but reducing the places that require changes makes sense.
wpears
left a comment
wpears
left a comment
There was a problem hiding this comment.
All aspects of this work as advertised. I played around with making a custom router and it seems like a nice feature!
willbarton
left a comment
willbarton
left a comment
There was a problem hiding this comment.
I agree with @wpears, this all looks great to me!
3 participants
Currently wagtail-sharing requires that the Wagtail sharing domain be configured in the Wagtail admin UI or via the SharingSite database table. This requires code to be run per environment to configure the sharing domain.
This PR introduces an alternate way of configuring the sharing domain via Django settings.
This is implemented in a new configurable routing system that allows users to plugin different routers that determine how wagtail-sharing works. The new
WAGTAILSHARING_ROUTERsetting can be used to select which router is used.The default approach continues to be database-based routing:
Alternatively, users can now specify settings-based routing:
Custom routers are now also supported, which should allow for different implementations such as token-based routing proposed in #47 (ping @zerolab).
All changes should be backwards compatible with existing deployments.
Please see the changes to the README for what should hopefully be comprehensive documentation of the new approach.