Custom admin site for reordering app list #2421
Conversation
|
Почини линтеры пожалуйста, а то кажется что мои замечания будут с ними пересекаться. Дай знать, если понадобится помощь с этим |
Как-то починил, но у меня пока вопрос не с линтером, а с тестами, как я вижу в ci прошло успешно, но локально не проходит тест, будто фикс для этого здесь, но у меня не работает #2339 |
|
А покажи выхлоп pytest, пожалуйста |
|
src/core/admin/admin_site.py
Outdated
| class AdminSite(admin.AdminSite): | ||
|
|
||
| def __init__(self, *args, **kwargs): | ||
| super(AdminSite, self).__init__(*args, **kwargs) |
There was a problem hiding this comment.
Это конструкция из второго питона, мы пишем на третьем
| super(AdminSite, self).__init__(*args, **kwargs) | |
| super().__init__(*args, **kwargs) |
src/core/admin/admin_site.py
Outdated
| def get_app_list(self, request: Any, app_label: Any | None = None) -> list[Any]: | ||
| app_list = super().get_app_list(request, app_label) | ||
| app_order = ["orders", "notion", "chains", "products", "otherapp"] | ||
| app_list.sort(key=lambda x: app_order.index(x["app_label"]) if x["app_label"] in app_order else len(app_order)) |
There was a problem hiding this comment.
На питоне так стараются не писать :-)
Попробуй переписать без лямбды
There was a problem hiding this comment.
Позволю себе немного пописать за тебя код, потому что хочу показать, что у нас в питоне есть много разных структур данных и операций над ними, с которыми приятно работать.
Твою задачу можно решить либо на set-ах, либо на списках и list comprehensions. Я бы решил её вот так:
def get_app_list():
sorted_app_list = ['orders', 'notion', 'test']
default_app_list = super().get_app_list()
sorted_app_list.extend([app for app in default_app_list if app not in sorted_app_list])
return sorted_app_listТут мы берём список приложений из задачи и добавляем в него все остальные приложения, которые нашла джанга. Читается гораздо легче, и кода меньше.
There was a problem hiding this comment.
Вот только app - это не строка, а какой-то dict с кучей полей :)
There was a problem hiding this comment.
Кек :-)
Тогда, если у тебя есть желание поупражняться в более питоничном коде — буду рад поревьюить. Если нет — то мне и так ок.
Можно даже отложить решение — смёрджить эту задачу сейчас, и дописать потом, когда будет желание и силы.
There was a problem hiding this comment.
Желание может и есть, но что-то пока элегантного решения в голову прийти не может, но потом бы я вернулся с новыми знаниями
src/core/admin/admin_site.py
Outdated
| super().__init__(name=name) | ||
| self._registry.update(admin.site._registry) | ||
|
|
||
| def get_app_list(self, request: Any, app_label: Any | None = None) -> list[Any]: |
There was a problem hiding this comment.
Вроде бы в django-stubs более жёсткая типизация. Может возьмём оттуда?
Я бы даже ещё ужесточил, и попробовал бы возвращать list[str]
There was a problem hiding this comment.
Я пока поменял на list[str], но сейчас подумал, что там же не str поидее, а какой-то объект, интерфейс которого нигде явно не описан, везде Any юзают
There was a problem hiding this comment.
Я думаю типа не описан просто потому, что никому из авторов django-stubs пока не приходилось кастомизировать это место админки :-)
There was a problem hiding this comment.
Здесь я на list[dict] поменяю, пожалуй, а то в ветке выше ты уже всё со строками поработать хочешь :)
|
На скриншоте проблема с локализацией. Почему-то у тебя на тачке включились переводы в тестах. Нет большого желания с этим разбираться — лучше сделать упавший тест более стабильным, зафиксировав для него английскую локаль как это сделано тут. |
| @@ -20,6 +20,11 @@ | |||
| ] | |||
|
|
|||
|
|
|||
| @pytest.fixture(autouse=True) | |||
There was a problem hiding this comment.
Мне хотелось бы видеть это в отдельном ПР, если у тебя есть время на это. Здесь мы работаем над списком приложений в админке
|
@f213 а можешь, пожалуйста, аппрув прожать, чтобы я точно был уверен, что могу мёрджить в текущем состоянии |
src/core/admin/admin_site.py
Outdated
| app_list.sort(key=self._get_app_order_index) | ||
| return app_list | ||
|
|
||
| def _get_app_order_index(self, element: Any) -> int: |
There was a problem hiding this comment.
Этот метод не использует состояние класс, поэтому его лучше оформить как @staticmethod, чтобы в будущем, когда мы забудем что он делает, было бы легче это восстановить.
src/core/admin/admin_site.py
Outdated
| app_list.sort(key=self._get_app_order_index) | ||
| return app_list | ||
|
|
||
| def _get_app_order_index(self, element: Any) -> int: |
There was a problem hiding this comment.
| def _get_app_order_index(self, element: Any) -> int: | |
| def _get_app_order_index(self, element: dict) -> int: |
Тут же вроде dict, не?
🏕️ https://3.basecamp.com/5104612/buckets/29069198/todos/7868529269
Добавил кастомный AdminSite, чтобы была возможность в нём сделать переопределение get_app_list и поменять сортировку для разделов и закинул его в urlpatterns вместо дефолтного, теперь в дашборде отсортировано как надо.
Сейчас если зайти в какую-нибудь модель, то в сайдбаре с аппками сортировка новая применяться не будет, я пока не могу понять, как подключить новый AdminSite для всего сразу, вообще мне хочется понять в том ли я направлению иду или какая-то другая реализация здесь нужна.
Есть ещё такой пакетик https://pypi.org/project/django-modeladmin-reorder/, но его втягивать побрезговал, хоть он вроде и рабочий до сих пор