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

ModelView discovery from models #207

Closed
jace opened this issue Dec 8, 2018 · 8 comments
Closed

ModelView discovery from models #207

jace opened this issue Dec 8, 2018 · 8 comments
Assignees

Comments

@jace
Copy link
Member

jace commented Dec 8, 2018

UrlForMixin combined with UrlForView allows a model to be queried for the URL to a view. However, we often need more metadata:

  1. Whether the view is available to the current user. At this time the logic for this is embedded into the template, making it difficult to maintain.

  2. A title, icon and category when displaying link buttons (once again hardcoded into the template at present).

  3. Enumeration of available views, or available related views.

Similar to model.url_for(action), we could have a model.view_for(action) that returns the ViewHandler from a ModelView affiliated with the model. View handlers have a data attribute that could be used by the template: {{ model.view_for('edit').data['title'] }}.

If #179 (requires_roles) is implemented using the ViewDecorator class, it can also offer an is_available property similar to that offered by StateTransition:

@property
def is_available(self):
"""
Property that indicates whether this transition is currently available.
"""
return not self._state_invalid()

A limited form of view discovery is available from the current_view proxy, which returns the current view class and allows examining its contents. There is a justifiable need for discovering ModelView classes that are not currently active.

@jace jace self-assigned this Dec 8, 2018
@jace
Copy link
Member Author

jace commented Feb 1, 2019

Models are currently overloaded with helper methods and data meant for views. This includes statemanager labels, transition metadata, and various methods and properties. These should really belong in a view, except we have the problem of being unable to discover these items from a template if they are placed anywhere other than the model.

This requirement goes beyond the view method discovery offered by Model.view_for, so perhaps we need another API:

  1. ClassViews can be registered to models using a simple invocation alongside the View.init_app call: Model.views.name = View. This could also be delegated to a metaclass in ModelView (using the Python symbol name, or the value of a __viewname__ attribute). This will require models to inherit from a base class that provides the views attribute. Alternatively, ModelView can provide such a method, inserting the necessary views attribute onto the model on first invocation.

  2. Access via the model class will return the view class: Model.views.name is ModelView

  3. Access via the instance will construct a view instance and initialise it so that obj.views.name.obj is obj. Repeated access during a concurrent request will return the same instance instead of constructing new instances each time.

Using this, a template can access a view helper method like this: obj.views.main.helper() where helper is guaranteed to receive the object as self.obj. (main is a suggested default view name, since most models will have a single view class).

@jace
Copy link
Member Author

jace commented Feb 5, 2019

This can be abstracted into a RegistryMixin that works for forms as well as views:

class RegistryMixin(object):
    """
	Provides the :attr:`forms` and :attr:`views` registries. Accessing
	a member of these registries via the class will return the member,
	while accessing via the instance will instantiate the member with
	an ``obj`` parameter.
	"""
	forms = Registry()
    views = Registry()

Unlike views, form instances are not safe to re-use, so the access protocol will have to change:

Model.forms.add_item('main', ModelForm)
Model.views.add_item('main', ModelView)

Model.views.main()  # Returns `ModelView`
model_instance.views.main()  # Returns `ModelView(obj=model_instance)`

There is no cache for instances. The caller is now responsible for this. The somewhat verbose keyword 'add_item' is reserved here and cannot be used as the name of an added item. It is chosen to avoid conflict with typical item names.

@jace
Copy link
Member Author

jace commented Feb 5, 2019

Model.views.main()  # Returns `ModelView`
model_instance.views.main()  # Returns `ModelView(obj=model_instance)`

This is a deviation from Pythonic syntax, where the variation between class and instance should be in the (), not in the container it is accessed from. This is better:

Model.views.main  # is `ModelView`
Model.views.main()  # Returns `ModelView()`
model_instance.views.main()  # Returns `ModelView(obj=model_instance)`
model_instance.views.main  # Returns a callable that must be called to get the instance

@jace
Copy link
Member Author

jace commented Feb 5, 2019

If we accept this latter syntax, then registering a new item could simply be:

Model.views.main = ModelView

No reserved keyword like add_item is necessary.

@jace
Copy link
Member Author

jace commented Feb 5, 2019

@vidya-ram @iambibhas question for you both: how do you anticipate this will be used in templates to access helper methods? This is still a somewhat crummy syntax:

{{ obj.views.main().helper() }}

Whereas if we place attributes directly on the model, we get:

{{ obj.helper() }}

@jace
Copy link
Member Author

jace commented Feb 6, 2019

Resolved in #215.

@jace jace closed this as completed Feb 6, 2019
@jace
Copy link
Member Author

jace commented Mar 4, 2019

It'll be nice to be able to do something like this:

{{ obj.views.main().edit.url_for() }}

This replaces or complements the view_for originally proposed.

However, the Funnel experience suggests this will be problematic. Funnel views work like this:

class BaseView(ModelView):
  pass  # All the views are here

@route('/main/app/path')
class AppView(BaseView):
    pass

AppView.init_app(app)
Model.views.main = AppView

@route('/legacy/app/path')
class FunnelView(BaseView):
    pass

FunnelView.init_app(app)
Model.views.funnel = FunnelView

Both views use the same templates, and the templates use obj.url_for(action), leaving it to the UrlForMixin mechanism to determine which classview is relevant for the current app. If the templates have a hard reference to class views, they will need additional logic to determine which classview to use. We're better off implementing the following two methods instead:

  1. Model.view_for(action='view'), returning the linked view handler
  2. Model.classview_for(action='view'), returning the class hosting the view handler

These methods could be added to RegistryMixin as they depend on its ability to create instances of the classview.

@jace
Copy link
Member Author

jace commented Mar 12, 2019

Closed in #219.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant