customizable identity attribute for ModelView URL generation#922
customizable identity attribute for ModelView URL generation#922x23n5902y wants to merge 5 commits intosmithyhq:mainfrom
Conversation
|
Hi, @aminalaee Thanks! |
|
@x23n5902y any updates? Or is it still failing because of other files? |
|
@x23n5902y could you fix this? I fails only on your patch, currently all tests pass |
mmzeynalli
added
waiting-for-feedback
mmzeynalli
left a comment
mmzeynalli
left a comment
There was a problem hiding this comment.
@x23n5902y could you also write tests for this? Like two modelviews of the same model, with different identity. Have different actions on each of them, and check they dont "see" each others actions.
|
|
||
| cls.pk_columns = get_primary_keys(model) | ||
| cls.identity = slugify_class_name(model.__name__) | ||
| cls.identity = slugify_class_name(attrs.get("identity", model.__name__)) |
There was a problem hiding this comment.
I think it would be better to do:
attrs.get("identity", slugify_class_name(model.__name__))
|
So, I found the problem. In this patch, 4 tests (same 2 tests in sync and async_view) fail: This happens because, in this patch, return request.url_for(
name,
identity=slugify_class_name(obj.__class__.__name__),
pk=get_object_identifier(obj),
)To return request.url_for(
name,
identity=self.identity,
pk=get_object_identifier(obj),
)Which works for most of the time, EXCEPT when building URL to FK objects, i. e., object from other model (in this case Address), and correspondingly, other identity. We cannot keep it as it was, because if Address model has user-defined identity, this will fail again. Upon looking for solutions and chatting with AI, I got this: # application.py:BaseAdmin:add_model_view
self._model_identity_map[view.model] = view.identity
# models.py
def _resolve_identity(self, obj: Any) -> str:
if isinstance(obj, self.model):
return self.identity
return self._admin_ref._model_identity_map.get(
obj.__class__, slugify_class_name(obj.__class__.__name__)
)
def _build_url_for(self, name: str, request: Request, obj: Any) -> URL:
return request.url_for(
name,
identity=self._resolve_identity(obj),
pk=get_object_identifier(obj),
)It is kind of an ugly solution, but it works. I am conflicted to fix this: it is really nice to have feature (I have needed it myself), but solution is ugly, and kind of hard to maintain. I will let @aminalaee to decide. |
mmzeynalli
added
needs-update
4 participants
Allow specifying a custom 'identity' attribute in ModelViewMeta for URL generation