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

feat: Выгрузка Хранилища значений в файлы #1488

Merged

Conversation

KrapivinAndrey
Copy link
Contributor

Сделанные изменения

  • Доработка механизма Подготовка и загрузка данных
  • В тестовые конфигурации добавлен реквизит с типом ХранилищеЗначения в спр. Товары
  • Тест на новый функционал

@KrapivinAndrey
Copy link
Contributor Author

run tests

@KrapivinAndrey
Copy link
Contributor Author

@DitriXNew

EndIf;

Path = ParamsValueStorage.PathToUpload
+ ?(Right(ParamsValueStorage.PathToUpload, 1) = "/", "", "/")
Copy link
Collaborator

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.

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

ну тогда может там сразу и сделать приведение к одному виду? Зачем это тянуть ниже. Оно же не один раз встречается

@@ -2274,7 +2389,7 @@ Procedure FormatGerkinTable(TableArray)
ParametersRow = "| ";
For Kkk = 0 To LengthArray.Count() - 1 Do
Ch = TrimAll(ParametersArray[Kkk]);
While StrLen(Ch) < LengthArray[Kkk] Do
While StrLen(Ch) < min(LengthArray[Kkk], 1024) Do
Copy link
Collaborator

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.

Это уже было

Copy link
Collaborator

Choose a reason for hiding this comment

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

ну так так код и рефакторится, кто то задел, кто то заметил, вот и качество растет :) Но судя по коду, числа раньше там не было, а это новый код.

@KrapivinAndrey
Copy link
Contributor Author

run tests

@KrapivinAndrey
Copy link
Contributor Author

run tests

@KrapivinAndrey
Copy link
Contributor Author

run tests

@@ -820,6 +827,31 @@ Procedure ReplaceRefByAttributeOnChange(Item)
ChangeReplaceRefByAttribute();
EndProcedure

&AtClient
Procedure PathToUploadНачалоВыбора(Item, Choose, StandardProcessing)
Copy link
Collaborator

Choose a reason for hiding this comment

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

если уж на инглише, то пусть будет все на инглишще :)

If Not (StrStartsWith(TypeName, "CatalogRef") Or StrStartsWith(TypeName, "DocumentRef") Or StrStartsWith(TypeName, "ChartOfCharacteristicTypesRef"))
Or DataValue.IsEmpty()
Or Dependencies.Find(DataValue, "Item") <> Undefined Then
If Not (Documents.AllRefsType().ContainsType(TypeVal)
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

EndProcedure

&AtServerNoContext
Function ParseStringValue(Val ParsingValue, Val ValueType)
Function ParseStringValue(Val ParsingValue, Val ValueType, workspaceRoot)
Copy link
Collaborator

Choose a reason for hiding this comment

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

workspaceRoot тоже везде пробрасывается, может его проще вынести тоже в экспортные? Зачем его везде передавать и про него помнить. Так получится сделать?

@@ -1528,6 +1563,17 @@ Function ParseStringValue(Val ParsingValue, Val ValueType)
Result = ReadXML(Reader);
Return Result;
EndIf;
If Left(ParsingValue, 17) = "ValueStoragePath:" Then
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

ну тут очевидно же. от увеличения переменной когнитивная нагрузка только вырастет. как ее назвать? КоличествоБуквВСтрокеValueStoragePathДвоеточие?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Reader.Close();

Return Result;
EndIf;
If Left(ParsingValue, 16) = "FindByAttribute:" Then
Copy link
Collaborator

Choose a reason for hiding this comment

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

магические числа

EndIf;

Path = ParamsValueStorage.PathToUpload
+ ?(Right(ParamsValueStorage.PathToUpload, 1) = "/", "", "/")
Copy link
Collaborator

Choose a reason for hiding this comment

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

ну тогда может там сразу и сделать приведение к одному виду? Зачем это тянуть ниже. Оно же не один раз встречается

@@ -2274,7 +2389,7 @@ Procedure FormatGerkinTable(TableArray)
ParametersRow = "| ";
For Kkk = 0 To LengthArray.Count() - 1 Do
Ch = TrimAll(ParametersArray[Kkk]);
While StrLen(Ch) < LengthArray[Kkk] Do
While StrLen(Ch) < min(LengthArray[Kkk], 1024) Do
Copy link
Collaborator

Choose a reason for hiding this comment

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

ну так так код и рефакторится, кто то задел, кто то заметил, вот и качество растет :) Но судя по коду, числа раньше там не было, а это новый код.

@DitriXNew
Copy link
Collaborator

В целом - круто, если не видишь смысла исправлять мои замечания - просто пометь их как закрытые и все.

@Pr-Mex
Copy link
Owner

Pr-Mex commented Jan 4, 2022

Да, согласен.

@KrapivinAndrey
Copy link
Contributor Author

Поправил замечания. И тесты

@KrapivinAndrey
Copy link
Contributor Author

run tests

@Pr-Mex
Copy link
Owner

Pr-Mex commented Jan 5, 2022

@KrapivinAndrey
Большая работа проделана. Спасибо!

@Pr-Mex
Copy link
Owner

Pr-Mex commented Jan 6, 2022

@KrapivinAndrey
Copy link
Contributor Author

@Pr-Mex Вечная проблема с / в КаталогПроекта и workspaceRoot

@Pr-Mex
Copy link
Owner

Pr-Mex commented Jan 6, 2022

Run tests

@KrapivinAndrey
Copy link
Contributor Author

@Pr-Mex надо перезапустить тесты

@KrapivinAndrey
Copy link
Contributor Author

Run tests

@Pr-Mex
Copy link
Owner

Pr-Mex commented Jan 8, 2022

@KrapivinAndrey
Тесты прошли!
Я тогда в понедельник ещё раз реквест весь целиком гляну и приму. А то сейчас с телефона.

@Pr-Mex
Copy link
Owner

Pr-Mex commented Jan 10, 2022

Спасибо!

@Pr-Mex Pr-Mex merged commit 2fb337f into Pr-Mex:develop Jan 10, 2022
@KrapivinAndrey KrapivinAndrey deleted the feature/binrary-for-preparetestdata branch January 10, 2022 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants