Процесс работ над релизом ocStore 1.5.5.1.2

[James026]

Хм действительно

[Bogdan1975]

foreach($limits as $limits){

Вы это осознанно пишите ...?

Это же полный бред!

[James026]

Сначала действительно кажется бредом...но

https://github.com/myopencart/ocStore/blob/master/catalog/controller/product/category.php

а должно быть видимо

https://github.com/opencart/opencart/blob/master/upload/catalog/controller/product/category.php

и

https://github.com/opencart/opencart/blob/master/upload/catalog/controller/product/special.php

 

 $this->data['limits'] = array();


 $limits = array_unique(array($this->config->get('config_catalog_limit'), 25, 50, 75, 100));


 sort($limits);


 foreach($limits as $value) {
    $this->data['limits'][] = array(
           'text' => $value,
           'value' => $value,
           'href' => $this->url->link('product/category', 'path=' . $this->request->get['path'] . $url . '&limit=' . $value)
     );
 }
 $url = '';

[freelancer]

лучше как-нить так

foreach($limits as $l){
  $this->data['limits'][] = array(
    'text'  => $l,
    'value' => $l,
    'href'  => $this->url->link('product/special', $url . '&limit=' . $l)
  );
}

[Alexey]

Нашёл баг.

Может быть его и легко исправить, но я не знаю как.

При редактировании категории, выпадающий список значений для "Родительская категория:"

сортируется по алфавиту, а не по заданному "Порядку сортировки"

 

Решено.

Контроллер из админки (admin/controller/catalog)

 

Как там говорят: "Критикуешь - предлагай. Предлагаешь - выполняй"  :ugeek:

 

Bogdan1975 - респект вам за инициативу, но ваше решение мягко говоря "не очень".

Не нужно писать дополнительную функцию сортировки и сортировать массив данных на стороне PHP, 

когда достаточно лишь правильно поменять сортировку в SQL запросе.

 

Я сегодня наконец то добрался до этого бага, пофиксил и сделал Pull-реквест. Вся суть изменений вот здесь:

https://github.com/myopencart/ocStore/pull/27/files

 

 

До одобрения Pull-реквеста Dinox-ом, для тех кто хочет уже сегодня получить багфикс - 

нужно извлечь содержимое прикреплённого архива в корень сайта.

fixSortingCategories.zip

[Bogdan1975]

Сначала действительно кажется бредом...но

Как можно что-то "вылечить" изменив переменную итератора?

Причем изменив так, что после выполнения цикла исходный массив будет испорчен.

[Alexey]

 

 

Может и бредовое предложение..... В карточке товара  есть не используемые поля  
 Артикул (SKU, код производителя):
UPC:
EAN:
JAN:
ISBN:
MPN:

 

Самый странный момент ,что заполнение этих полей не приведёт к их появлению в карточке товара.

Отсюда предложение.

Сделать условие на вывод их в карточке товара если эти поля заполнены.А теперь поясню для чего.Простым переименованием можно добавить  6 новых пунктов в карточку товара.

 

я поддерживаю Тома, логично так-то, иногда приходиться добавлять дополнительные характеристики к товару с выводом в главном блоке, а так по дефолту уже 5, просто названия требуемые подставить

Пардон, но это очень бредовое предложение!  :(

 

Я согласен с afwollis:

и красный цвет называть зеленым.
а чо - кому-то удобно.

Tom, переименование - вредные советы.

 

и с freelancer:

лучше стилями скрыть чем удалять.

Потому что всё правильно они говорят! 

В ядро нужно вносить минимальные изменения — только то, что действительно очень критично и очень важно. 

 

Одному будет удобней переименовать красный цвет в зелёный, другому будет удобней переименовать красный цвет в табуретки, третьему будет удобней переименовать красный цвет в "плюшевые мишки". Ну бред же!  :ugeek:

 

Если кому то надо дополнительные поля - то во всех отношениях будет лучше самостоятельно их добавить для своего магазина, причём с соответствующими названиями полей в базе данных, соответствующими названиями переменных и т.д. Это очень поможет не путаться при дальнейшей доработке своего магазина.

 

Я вот каждый раз скрываю (не удаляю для совместимости) везде отображаемое поле "количество" и  не переименовываю какое-то из существующих, а добавляю новое "цена производителя" и ничего, не жалуюсь.

Изменения минимальны и понятны, обновлятся легко.  :-)

 

Потому что: каждая кастомизация — мешает обновлять движок ocStore при выходе новых версий оригинального Opencart. 

И если эта кастомизация нужна не всем, то проще её сделать в ввиде отдельного модуля чем добавлять в ядро.

 

Поскольку добавление в ядро не всем нужной фичи — создаёт лишний гемморой для всего сообщества при разработке всех следующих версий ocStore на базе новых версии оригинального Opencart.

[Bogdan1975]

Bogdan1975 - респект вам за инициативу, но ваше решение мягко говоря "не очень".

Не нужно писать дополнительную функцию сортировки и сортировать массив данных на стороне PHP, 

когда достаточно лишь правильно поменять сортировку в SQL запросе.

Очевидно, мы просто разные задачи перед собой ставили.

Я выполнял такую задачу: вначале родительская категория с наименьшим значением sort_order, ниже дочерняя категория с наименьшим значением sort_order, ниже "субдочерняя" и т.д., независимо от уровня вложенности.

Для этого нужна рекурсия. Если такой рекурсивный запрос и можно сделать, то у меня как минимум для этого познаний в SQL не хватает. Но я и не парился, т.к. даже если есть возможность выполнить эту рекурсию средствами SQL, то на 99% уверен, что php с этим справится на порядок быстрее.

Предложенное Вами решение решает другую задачу - выдает категории отсортированные по sort_order, не структурируя их по уровням вложенности.

[Alexey]

Очевидно, мы просто разные задачи перед собой ставили.

Я выполнял такую задачу: вначале родительская категория с наименьшим значением sort_order, ниже дочерняя категория с наименьшим значением sort_order, ниже "субдочерняя" и т.д., независимо от уровня вложенности.

Для этого нужна рекурсия. Если такой рекурсивный запрос и можно сделать, то у меня как минимум для этого познаний в SQL не хватает. Но я и не парился, т.к. даже если есть возможность выполнить эту рекурсию средствами SQL, то на 99% уверен, что php с этим справится на порядок быстрее.

Предложенное Вами решение решает другую задачу - выдает категории отсортированные по sort_order, не структурируя их по уровням вложенности.

Так а чтобы не ломать себе голову как бы так "структурировать по уровням вложенности", всего то нужно в категориях правильный sort_order проставить.

У меня родительские категории первого уровня имеют sort_order

100

200

...

800

 

для родительской категории с sort_order 200 вложенные имеют sort_order:

210

220

..

270

 

Третий уровень вложенности (если есть) для категории с sort_order 210:

211

212

213

...

219

 

Вообще, есть sort_order - по нему и нужно выдавать список категорий. Везде так и есть, а в том месте отсортировать забыли. Вот я и пофиксил.

[Alexey]

Нашел решение с отключением не нужных (пока) языков, если они есть в базе данных.

В файл модели   admin\model\localisation\language.php

добавил функцию getLanguagesBD. Она копия изначальной функции getLanguages.

А в getLanguages вставил фильтр на status='1'

стр. 297

было
$query = $this->db->query("SELECT * FROM " . DB_PREFIX . "language ORDER BY sort_order, name");
новое
$query = $this->db->query("SELECT * FROM " . DB_PREFIX . "language WHERE status='1'   ORDER BY sort_order, name"); // фильтр на не активные языки

К getLanguagesBD обращается только контроллер   admin\controller\localisation\language.php

для получения данных о языках в базе данных , в котором в строке 175                         

$results = $this->model_localisation_language->getLanguages($data);

заменено на                            

 $results = $this->model_localisation_language->getLanguagesBD ($data);

Теперь, если на вкладке "Языки" перейти по "изменить" и установить в Отключено  "Статус:

Показывать/Скрывать в переключателе языков витрины магазина" , то в Админке не будет табов с не активными языками.

Теперь не нужно обязательно заполнять еще один язык - он не показывается.

Вообще то, надо сделать отдельный выключатель, чтобы не привязываться к отображению языка в магазине.

Ну да, в админке не будет табов с отключенными языками, а дальше что? Вы включите язык, который раньше был отключен и как будут показыватся товары, для которых вы не прописали тексты?

 

Если вам точно не нужен язык ни сейчас ни в будущем - тогда удаляйте его и не будет у вас лишних табов для него. А если вы всего лишь временно отключили язык - то всё там правильно, эти табы нужны, так как в таком случае тексты обязательно нужно прописывать.

 

Так что, ничего не трогайте, в этом плане там всё правильно сделано.

[Bogdan1975]

Alexey сказал(а) 04 Ноя 2013 - 6:47 PM:

Я вот каждый раз скрываю (не удаляю для совместимости) везде отображаемое поле "количество" и не переименовываю какое-то из существующих, а добавляю новое "цена производителя" и ничего, не жалуюсь.

Кстати, о совместимости. Есть легкая несовместимость между 1.5.4.1 и 1.5.5.1 в плане переменных.

Например, в header то что в 1.5.4.1 "filter_name", то в 1.5.5.1 - "search". В результате - несовместимость шаблонов.

Есть мысль, что бы отловить подобные моменты и в контроллерах продублировать выдачу и под 1.5.4.1 и под 1.5.5.1 ?

С одной стороны не хорошо, что лишние переменные, с другой - классно, что можно безболезненно ставить шаблоны под 1.5.4.1 (ну и более ранние, на сколько версий - не знаю).

Какие есть мнения на счет этого?

[Bogdan1975]

Alexey сказал(а) 04 Ноя 2013 - 7:08 PM:

Так а чтобы не ломать себе голову как бы так "структурировать по уровням вложенности", всего то нужно в категориях правильный sort_order проставить.

Структурно отсортированные категории позволяют не париться алгоритмом присвоения sort_order.

Ну это не так важно. 2 решения лучше, чем ни одного :)

Как генералы решат, так и будет.

[Alexey]

Кстати, о совместимости. Есть легкая несовместимость между 1.5.4.1 и 1.5.5.1 в плане переменных.

Например, в header то что в 1.5.4.1 "filter_name", то в 1.5.5.1 - "search". В результате - несовместимость шаблонов.

Есть мысль, что бы отловить подобные моменты и в контроллерах продублировать выдачу и под 1.5.4.1 и под 1.5.5.1 ?

С одной стороны не хорошо, что лишние переменные, с другой - классно, что можно безболезненно ставить шаблоны под 1.5.4.1 (ну и более ранние, на сколько версий - не знаю).

Какие есть мнения на счет этого?

Мнение такое, что поддержка совместимости шаблона с новой версией - это сфера отвественности автора шаблона. 

Иначе, следуя вашей логике можно дойти до того, что нужно поддерживать шаблоны разработанные для Opencart 1.5.3.1 и т.д.

 

 

Структурно отсортированные категории позволяют не париться алгоритмом присвоения sort_order.

Ну это не так важно. 2 решения лучше, чем ни одного :)

Как генералы решат, так и будет.

Всего один раз "попарившись" - чтобы правильно задать sort_order (задача для первоклассника, если честно) 

У нас прямо в результате запроса к БД - сразу получаются категории в нужном порядке.

 

А так как это делается на уровне SQL и без рекурсии - то это выигрыш в производительности.

Пускай незначительный выигрыш, но из таких вот небольших составляющих - складывается быстродействие всей системы в целом.

[James026]

Bogdan1975 , я к тому что:

Во-первых: почему заминусовали человека, ведь врят ли он сам дошёл до такого решения, а просто посмотрел рядом лежащие файлы.

Во-вторых: без его замечания, хз сколько бы прошло времени, пока не заметили эту ошибку бы, причём в репах ocStore она есть, в репах Opencart там всё исправлено(я привёл пример ссылок).

Ну а то что массив перебирается массивом...ну ведь кто-то же и в репо это закомитил) :-)

[Bogdan1975]

Alexey сказал(а) 04 Ноя 2013 - 7:17 PM:

Если вам точно не нужен язык ни сейчас ни в будущем - тогда удаляйте его и не будет у вас лишних табов для него. А если вы всего лишь временно отключили язык - то всё там правильно, эти табы нужны, так как в таком случае тексты обязательно нужно прописывать.

Можно в принципе при включении языка делать также как и при добавлении языка - копировать данные из существующего языка.

В принципе проблема реальная, но скорее всего табы нужны, но убрать бы валидацию на пустые данные в отключенном языке.

Ситуация простая. Есть желание делать 2-язычный магазин, но нет возможности делать это сразу. Удобней всего отключить 2-й язык и наполнять его данными основного языка. А уже после запуска, наполнить данными второго языка. А для такой ситуации обязательность данных отключенного языка не подходит, скрытие таба - тоже.

[Bogdan1975]

James026 сказал(а) 04 Ноя 2013 - 7:47 PM:

Bogdan1975 , я к тому что:

Во-первых: почему заминусовали человека, ведь врят ли он сам дошёл до такого решения, а просто посмотрел рядом лежащие файлы.

Ну это вопрос к минусовавшим

James026 сказал(а) 04 Ноя 2013 - 7:47 PM:

Bogdan1975 , я к тому что:

Во-вторых: без его замечания, хз сколько бы прошло времени, пока не заметили эту ошибку бы, причём в репах ocStore она есть, в репах Opencart там всё исправлено(я привёл пример ссылок).

Ну а то что массив перебирается массивом...ну ведь кто-то же и в репо это закомитил) :-)

Если внимательно прочитать пост, то он предлагает решить "проблему" (я так и не понял в чем там проблема) посредством изменения "$limits as $limit" на "$limits as $limits". Если бы наоборот, то хоть что-то здравое в этом было бы (хотя и не решало бы ничего, массив все равно переберется), а так ...

[Bogdan1975]

Alexey сказал(а) 04 Ноя 2013 - 7:42 PM:

Иначе, следуя вашей логике можно дойти до того, что нужно поддерживать шаблоны разработанные для Opencart 1.5.3.1 и т.д.

Честно говоря, мысли вплотную к этому подкрались :). Все же любые вопросы совместимости на мой взгляд являются плюсом (ну смотря какой ценой, конечно).

Но сомнения в их правильности есть, поэтому и интересно мнение сообщества.

 

Alexey сказал(а) 04 Ноя 2013 - 7:42 PM:

А так как это делается на уровне SQL и без рекурсии - то это выигрыш в производительности.

Пускай незначительный выигрыш, но из таких вот небольших составляющих - складывается быстродействие всей системы в целом.

Тут уж возразить нечего Таки есть что возразить

Змінено 4 листопада 2013 користувачем Bogdan1975

[Alexey]

Можно в принципе при включении языка делать также как и при добавлении языка - копировать данные из существующего языка.

В принципе проблема реальная, но скорее всего табы нужны, но убрать бы валидацию на пустые данные в отключенном языке.

Ситуация простая. Есть желание делать 2-язычный магазин, но нет возможности делать это сразу. Удобней всего отключить 2-й язык и наполнять его данными основного языка. А уже после запуска, наполнить данными второго языка. А для такой ситуации обязательность данных отключенного языка не подходит, скрытие таба - тоже.

Неправильный алгоритм. В той ситуации что вы описываете, нужно сделать так.

1.Добавляем один язык, стартуем магазин.

2.Пришло время добавить второй язык - добавляем его (при этом тексты автоматически копируются из старого языка в новый)

3.Отключаем новый язык в админке. Он не показывается, но при редактировании товара есть табы с текстами нового языка

4.Редактируем в админке названия и описания товаров для нового языка для всех товаров. Внешне этого не видно, работа идёт "внутри", то есть в админке.

5.Когда всё отредактировали - включаем новый язык и он начинает показыватся на сайте.

[Bogdan1975]

нужно сделать так.

1.Добавляем один язык, стартуем магазин.

2.Пришло время добавить второй язык - добавляем его (при этом тексты автоматически копируются из старого языка в новый)

Согласен. Действительно, нет смысла чего-то воротить.

[Alexey]

Честно говоря, мысли вплотную к этому подкрались :). Все же любые вопросы совместимости на мой взгляд являются плюсом (ну смотря какой ценой, конечно).

Но сомнения в их правильности есть, поэтому и интересно мнение сообщества.

Согласитесь, что совместимость с оригинальным Opencart - для нас важнее, чем совместимость с шаблонами для старых версий. В оригинальном Opencart-е нет тех переменных, которые вы указали. Соответственно и в ocStore должно быть точно так же. А шаблоны для старых версий - пускай авторы шаблонов обновляют. Это их сфера ответственности и не зря же, они берут деньги и обещают поддержку совместимости с будущими версиями.

[Bogdan1975]

Если внимательно прочитать пост, то он предлагает решить "проблему" (я так и не понял в чем там проблема) посредством изменения "$limits as $limit" на "$limits as $limits". Если бы наоборот, то хоть что-то здравое в этом было бы (хотя и не решало бы ничего, массив все равно переберется), а так ...

Просмотрев весь код, а не только его часть, вынужден признать свою неправоту и принести извинения перед NickZet

"$limits as $limit" несет в себе проблему так как портит ранее установленную (и нужную) переменную $limit.

"$limits as $limits" проблем в себе не несет, т.к. далее переменная $limits не используется, но все равно как-то неправильно такие конструкции использовать.

[Bogdan1975]

Так а чтобы не ломать себе голову как бы так "структурировать по уровням вложенности", всего то нужно в категориях правильный sort_order проставить.

У меня родительские категории первого уровня имеют sort_order

100

200

...

800

 

для родительской категории с sort_order 200 вложенные имеют sort_order:

210

220

..

270

 

Третий уровень вложенности (если есть) для категории с sort_order 210:

211

212

213

...

219

А потом у меня появится еще 1 товарная группа (главная категория), и мне нужно чтобы она была между 100 и 200. Как быть?

У меня сортировка внутри категории/подкатегории выглядит как 10, 20, 30 и т.д. И не для того чтобы дочерним назначать 11, 12; 21, 22, а для того, что бы была возможность безболезненно вставлять категории внутри уровня.

Не думаю, что я такой "эксклюзивный", возможно, часть пользователей нумеруют по такому же принципу.

Т.е. безструктурная сортировка фактически обязывает нумеровать по принципу, используемым Вами, хотя не факт что он идеален. А если не соблюдать "алгоритм" нумерования, то я и мне подобные на выходе будут получать результат ничем не лучше, чем просто неотсортированный

 

Всего один раз "попарившись" - чтобы правильно задать sort_order (задача для первоклассника, если честно)

На практике сталкивался с магазином, в котором в 1-й главной всего 1 подуровень, но там .. 1500 подкатегорий, во 2-й главной все подкатегории короткие, но до 5 уровней вложенности. Понимаю, что сама организация категорий таким образом - не самый правильный вариант, но так есть. Организовать sort_order по Вашему принципу, конечно, возможно, но не так уж и просто, и точно не так уж удобно.

 

У нас прямо в результате запроса к БД - сразу получаются категории в нужном порядке.

Не уверен, что сортировка в sql по неиндексированным полям даст какой-то выигрыш. Вполне вероятно, что рекурсия в php отработает быстрее, чем простая сортировка по sort_order (в таблице category нет индекса по sort_order). Хотя тут, конечно, скорее всего многое еще будет зависить от того, что из себя представляет дерево категорий. В случае магазина со странной структурой категорий (который я привел в пример), наверное все же рекурсия будет медленнее.

 

Поэтому, все же предлагаю дать право на жизнь и рассмотреть вариант "структурной" (рекурсивной) сортировки.

[Alexey]

Поэтому, все же предлагаю дать право на жизнь и рассмотреть вариант "структурной" (рекурсивной) сортировки.

 

Стандартно, нужно сортировать по sort_order. Это общепринято.

Если, как вы утверждаете, рекурсивная сортировка имеет право на жизнь - предлагаю вам сделать её в виде дополнительного модуля.

Если кому то будет нужна именно такая, нестандартная "рекурсивная" сортировка категорий - будут пользоваться вашим модулем.

[Bogdan1975]

Стандартно, нужно сортировать по sort_order. Это общепринято.

Если, как вы утверждаете, рекурсивная сортировка имеет право на жизнь - предлагаю вам сделать её в виде дополнительного модуля.

Если кому то будет нужна именно такая, нестандартная "рекурсивная" сортировка категорий - будут пользоваться вашим модулем.

Блиииин ....

Чего ж я сразу не глянул!

Alexey, гляньте на сборку ocStore 1.5.4.1 - тогда было "общепринято" сортировать рекурсивно!!!

А мы тут спорим ...

Нам с Вами должно быть стыдно, причем Вам - вдойне :) (шутка)

В 1.5.4.1 все нормально сортировалось (рекурсивно, кстати, а не как принято)

В 1.5.5.1.1 ничего этого уже нет, рекурсивная функция отсутствует, категории забираются скопом и без сортировки отдаются в мир.

В мастер-версии все восстановленно, но одна строчка все портит.

А по сему, изобретенные мною с Вами велосипеды нужно выкинуть на помойку (а лучше сжечь), а вместо этого в мастер-версии фрагмент:

        // Categories
        $this->load->model('catalog/category');
        $this->data['categories'] = $this->model_catalog_category->getCategories(0);

нужно отправить вслед за велосипедами.

Хотя нет .... "// Categories" нужно оставить :)

Спешка и невнимательность - вот причины, по которым мы с Вами тут холиварить начали

[Alexey]

        // Categories
        $this->load->model('catalog/category');
        $this->data['categories'] = $this->model_catalog_category->getCategories(0);

нужно отправить вслед за велосипедами.

Хотя нет .... "// Categories" нужно оставить :)

Спешка и невнимательность - вот причины, по которым мы с Вами тут холиварить начали

Вы так и не раскрыли суть и я, если честно, ничего не понял из этого сообщения.  :ugeek:

Насчёт холивара - то никакого холивара не было. Ваше первоначальное решение - было мягко говоря "кривоватым", и я поправил 

сортировку в SQL а вы с невероятным усердием начали доказывать "право на жизнь" своего метода. Ну да ладно, если в 1.5.4.1

- сортировка делалась по другому, то что ж вы не написали пост, прочитав который, было бы понятно как работал код сортировки категорий в 1.5.4.1 ?

 

 

Из хороших новостей, проблему по которой были вот эти все сообщения:

 

Небольшой баг: на странице "Акции" не сортируются товары по количеству (всегда стоит 100). Можно внести в todo list.

Этот баг наблюдается и в демке. Лечится просто добавлением буквы s в файле catalog\controller\product\special:

 

$this->data['limits'] = array();

 

$limits = array_unique(array($this->config->get('config_catalog_limit'), 25, 50, 75, 100));

 

sort($limits);

 

foreach($limits as $limits){

$this->data['limits'][] = array(

'text'  => $limits,

'value' => $limits,

'href'  => $this->url->link('product/special', $url . '&limit=' . $limits)

);

}

Вы это осознанно пишите ...?

Это же полный бред!

Сначала действительно кажется бредом...но

https://github.com/myopencart/ocStore/blob/master/catalog/controller/product/category.php

а должно быть видимо

https://github.com/opencart/opencart/blob/master/upload/catalog/controller/product/category.php

и

https://github.com/opencart/opencart/blob/master/upload/catalog/controller/product/special.php

 

 $this->data['limits'] = array();

 $limits = array_unique(array($this->config->get('config_catalog_limit'), 25, 50, 75, 100));

 sort($limits);

 foreach($limits as $value) {

    $this->data['limits'][] = array(

           'text' => $value,

           'value' => $value,

           'href' => $this->url->link('product/category', 'path=' . $this->request->get['path'] . $url . '&limit=' . $value)

     );

 }

 $url = '';

Как можно что-то "вылечить" изменив переменную итератора?

Причем изменив так, что после выполнения цикла исходный массив будет испорчен.

 

Я исправил сегодня вот этим Pull-реквестом

https://github.com/myopencart/ocStore/pull/28/files

 

Баг был в том, что создание переменной-итератор цикла $limit  - перезатирало внешнюю переменную $limit.

 

В special.php, я исправил переменную-итератор на $value (по аналогии с оригинальным Opencart) 

 

И, хоть бага не было, но для порядка отрефакторил $limits на $value, в этих файлах:

category.php

manufacturer.php

search.php

 

Прикреплённый архив делать не буду. Уговаривайте Dinox-a, 

чтобы он наконец-то принял мои правки, уже 6 Pull реквестов висит.

https://github.com/myopencart/ocStore/pulls

 

После того, как Dinox примет мои правки, пре-релиз ocStore со всеми багфиксами можно будет скачать тут:

https://github.com/myopencart/ocStore/archive/master.zip