Conversation
There was a problem hiding this comment.
Добрый день!
Спасибо за PR. Статья требовала переработки, и направление мы забираем почти целиком.
Что пригодилось:
- переход с
new Query(BookTable::getEntity())наBookTable::query()— обоснование черезgetQueryClass()корректное, проверила по ядру; - сравнение трёх подходов к записи одного запроса;
- разбивка на
###вместо жирных псевдозаголовков — разделы попадут в оглавление страницы; - примеры к группам методов вместо голого перечня;
- исправленная дата в SQL-комментарии — эта опечатка жила в статье давно.
Оставила 13 замечаний по строкам. Блокирующих пять: три фактические ошибки (поля объекта, имя таблицы, результат hasDistinct) и две по разметке, которые могут сломать вёрстку (вкладки и тип плашки note).
Ориентиры: правила оформления примеров кода — в CONTRIBUTING.md, образец оформления статьи для этого раздела — соседняя pages/orm/querying-data.md. Она задаёт вид списков, заголовков, подводок и плашек.
В двух комментариях (строки 55 и 240) я сразу дала готовый текст со ссылками на исходники: там нужна сверка с ядром, и проверять это вам не нужно.
|
Спасибо за ревью - достаточно подробно и информативно получилось. Действительно, многие фрагменты нуждались в переработке и упрощении текста, поэтому я пошел еще чуть дальше - прогнал скилом от "Пиши сокращай" чтобы улучшить результат. |
ech-bitrix-doc
left a comment
ech-bitrix-doc
left a comment
There was a problem hiding this comment.
Спасибо, большую часть правок применили точно.
Отдельно про прогон через «Пиши, сокращай» — он реально пошёл статье на пользу, и в нескольких местах ваш вариант лучше моего. Возвращать мои формулировки не надо
Оставшиеся пункты указаны в комментариях
| Если параметры запроса формируются программно, используйте построитель запросов — объект `Bitrix\Main\ORM\Query\Query`. Построитель накапливает параметры и выполняет запрос по вызову метода `exec`. | ||
|
|
||
| Пример с `getList` | ||
| Сравните три способа собрать один и тот же запрос на получение книги по идентификатору. Все примеры статьи используют класс `BookTable` — его описание смотрите в статье [Операции с сущностями](./entity-operations.md). |
There was a problem hiding this comment.
Якорь потерялся. В entity-operations.md:445 есть заголовок ## Пример класса BookTable {#booktable-example} — ссылка должна вести на него, а не на статью целиком.
Замените строку на:
Сравните три способа собрать один и тот же запрос на получение книги по идентификатору. Все примеры статьи используют объект BookTable — его класс целиком смотрите в статье Операции с сущностями.
|
|
||
| Создавайте объект `Query` методом `query()` нужной таблицы, а не через `new Query()`. Метод `query()` возвращает класс запроса, указанный в методе `getQueryClass()` таблицы. Например, модуль информационных блоков подставляет свой класс запроса. Вызов `new Query()` всегда создает базовый класс, поэтому доработки таблицы теряются. | ||
|
|
||
| {% endnote %} |
There was a problem hiding this comment.
После плашки потерялся факт, который был в статье до PR — в main это строка 32: переопределение getList работает не всегда. Плашке он не противоречит, а дополняет ее.
Добавьте абзац после {% endnote %}:
Переопределенный метод getList срабатывает не всегда. Если запрос собирают через объект Query и выполняют методом exec, вызова getList не происходит и переопределение не применяется.
| - `set` заменяет ранее заданное значение. | ||
|
|
||
| - Условие внутри функции добавляет поле `ISBN`, если оно необходимо | ||
| - `add` дополняет его. | ||
|
|
||
| **Добавление фильтров и сортировки**. Функция `attachOthers` добавляет фильтры и сортировку. | ||
| - `get` возвращает текущее значение. |
There was a problem hiding this comment.
Маркеры списков идут с одним пробелом. В разделе принято два
Поправьте во всех 17 пунктах: строки 125, 127, 129, 139, 141, 143, 174, 176, 199, 201, 203, 242, 244, 246, 262, 264, 283.
| - `set` заменяет ранее заданное значение. | |
| - Условие внутри функции добавляет поле `ISBN`, если оно необходимо | |
| - `add` дополняет его. | |
| **Добавление фильтров и сортировки**. Функция `attachOthers` добавляет фильтры и сортировку. | |
| - `get` возвращает текущее значение. | |
| - `set` заменяет ранее заданное значение. | |
| - `add` дополняет его. | |
| - `get` возвращает текущее значение. |
| $sql = $q->getQuery(); | ||
| file_put_contents('/tmp/today_books.sql', $sql); | ||
| // Запрос "SELECT ID FROM my_book WHERE PUBLISH_DATE='2014-12-31'" будет сохранен в файл, но не выполнен. | ||
| Чтобы посчитать книги по датам выхода, добавьте группировку. Поле `CNT` описывает объект `ExpressionField` — о таких полях рассказывает раздел [Runtime-поля](#runtime-polya). |
There was a problem hiding this comment.
Ссылка не разрешится. Якорь генерируется из заголовка и кириллицу сохраняет, а явного {#runtime-polya} у заголовка нет. Проверить можно по focus-monitor.md:39 — там ссылка #включить-логирование ведет на заголовок ## Включить логирование без явного якоря.
Нужно либо [Runtime-поля](#runtime-поля), либо задать заголовку явный якорь: ### Runtime-поля {#runtime-fields} и ссылку на #runtime-fields.
| @@ -71,73 +97,223 @@ function attachOthers(Query $query): void | |||
| } | |||
| ``` | |||
There was a problem hiding this comment.
Замечание было про первое лицо и точки в пунктах, а не про то, что разбор примера лишний. Сейчас блок кода со строк 64–98 остался без пояснения.
Замените строку на:
Создание объекта Query. Метод BookTable::query() создает объект Query, связанный с таблицей книг. Объект становится основой для построения запроса.
Добавление полей в запрос. Функция attachSelect добавляет поля, которые нужно выбрать из базы данных.
-
addSelect('ID')добавляет полеIDв список выбираемых полей. -
Условие внутри функции добавляет поле
ISBN, если оно необходимо.
Добавление фильтров и сортировки. Функция attachOthers добавляет фильтры и сортировку.
-
setFilterустанавливает условия фильтрации данных. -
setOrderзадает порядок сортировки результатов.
2 participants
Я посмотрел последний PR с изменением namespace и мне показалось важным дополнить эту статью с актуальными примерами и показать что это немного больше чем просто Query объект и он не такой уж сложный - нет необходимости в оборачивании Datamanager-наследника.