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

Нумерация заказа MS2 #393

Merged
merged 11 commits into from
Jun 30, 2020

Conversation

CrazyBoy49z
Copy link
Contributor

Что оно делает?

#370

Внимание код не тестировал нужно, проверить, по логике должно работать. Переводи не делал

@Ibochkarev Ibochkarev added this to the 2.5.1 milestone Apr 26, 2020
@webinmd
Copy link
Collaborator

webinmd commented May 4, 2020

Протестировал, системная настройка ms2_order_format_num_separator никак не участвует в формировании номера заказа. Там всегда получаем "/"

1dcc39b#diff-081fcf6573330c008ba49f7c4731cdf4R600

Ну и в лексиконах есть ошибки:
Разделитель для нумерации заказа. Доступние значения "/" и ","
Ошибка тут: Доступние - Доступные

И тут же если добавляется в разделитель символ минуса, то и в описание про него надо добавить.

Copy link
Member

@Ibochkarev Ibochkarev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Прошу поправить по комментариям Евгения. Спасибо

Изменил раздел настроек
Поправил описание
@OlegShchavelev
Copy link

Изменение года работает, изменил настройку ms2_date_format значение %Y

изображение

в итоге номер заказа изменился

изображение

Изменение префикса не работает ms2_order_format_num_separator поставил значение -

изображение

получаем

изображение

@OlegShchavelev
Copy link

OlegShchavelev commented May 6, 2020

Я предлагаю обсудить следующий вариант реализации. В варианте который предложил @CrazyBoy49z настройка отвечает за определенное действие. Если посмотреть на ISSUE (ссылка на него есть в PR), то можно представить сколько будет в итоге настроек если выполнить ISSUE на 100%.

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

К примеру:
{"prefix":"MS2-","date":"%Y","separator":'"-"}

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

Тем самым мы выполняем задачу №2 и №3 а задачу №1 можно перенести на следующию версию.

@CrazyBoy49z
Copy link
Contributor Author

Спасибо парни, исправлю по возможности

@webinmd
Copy link
Collaborator

webinmd commented May 7, 2020

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

К примеру:
{"prefix":"MS2-","date":"%Y","separator":'"-"}

Давайте не усложнять этот функционал, сейчас никто не запрещает добавить префис перед датой, он точно также работает. Я проверял этот момент.
ТО есть если вписать не просто %Y а MS2-%Y то в номер заказа всё записывается.

Осталось сделать так чтобы разделитель подставлялся указанный и всё.

@CrazyBoy49z
Copy link
Contributor Author

Осталось сделать так чтобы разделитель подставлялся указанный и всё.

Нашёл ошибку у себя в коде, только освобожусь исправлю

Copy link
Member

@alroniks alroniks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@@ -596,7 +596,15 @@ public function getCost($with_cart = true, $only_cost = false)
*/
public function getNum()
{
$cur = date('ym');
$formatNum = htmlspecialchars($this->modx->getOption('ms2_order_format_num', null, '%y%m'));
$formatNumSeparator = trim(preg_replace("/[^,\/\-]/", '', "\/"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Нужно из опции тоже вытаскивать, а то опция как бы есть, но бесполезна.

$num = explode('/', $num);
$num = $cur . '/' . ($num[1] + 1);
$num = explode($formatNumSeparator, $num);
$num = "{$cur}{$formatNumSeparator}" . ($num[1] + 1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Дикое переопределение переменных. Можно переписать чуть более наглядно.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Поправлю только, доберусь, ошибки есть так набросал в браузере, я б совсем завел новое поле в таблице на количество, но думаю это уже нужно будет добавить в minishop3

@Ibochkarev
Copy link
Member

@DetaliDigital прошу еще раз протестировать.

@Ibochkarev Ibochkarev merged commit 3b888e6 into modx-pro:master Jun 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants