Implementation/58105 item persistence service #16884
Conversation
|
Caution The provided work package version does not match the core version Details:
Please make sure that:
|
| it "creates a failure result" do | ||
| [ | ||
| { parent:, label: "A label", short: "" }, | ||
| { parent:, label: "A label", short: nil }, |
There was a problem hiding this comment.
🔴 we might have a problem with that case. If I interpret the usage in the service correct, it will call it, when short is not passed.
Solutions:
a) adapt contract, so that passing nil is allowed, while "" still is forbidden.
b) adapt service to call contract only with non-nil shorts or no short key at all.
To make b) work we would need to create a hash from the method parameters before putting them into the call. something like:
input = {parent:, label:, short:}.compact
CustomFields::Hierarchy::InsertItemContract
.new
.call(**input)There was a problem hiding this comment.
short is indeed passed, having short: nil in the hash actually creates the key in the hash. Therefor you have three cases: short: "", short: nil and no short.
There was a problem hiding this comment.
discussed personally, there is indeed a faulty call from the service. Another test case discovered that.
| end | ||
|
|
||
| rule(:hierarchy_root) do | ||
| key.failure("Hierarchical root already set") unless value.nil? |
There was a problem hiding this comment.
🟡 change to if value.present? to battle this godless unless :D
| end | ||
|
|
||
| rule(:field_format) do | ||
| key.failure("Custom field must have field format 'hierarchy'") unless value == "hierarchy" |
There was a problem hiding this comment.
🟡 same here: can we use if value != "hierarchy"?
| end | ||
| end | ||
|
|
||
| context "with invalid hierarchy root" do |
There was a problem hiding this comment.
❓ I don't understand the test case. So, is this an item object which cannot get persisted?
Maybe renaming it to "if persistence of hierarchy root fails"? Otherwise some transitional head work is needed when reading the test - as in "an invalid hierarchy root will lead to a persistence error". But this is no hill I gonna die upon.
Ticket
OP#58105
What are you trying to accomplish?
PART 1
Implement a service capable of manipulation a hierarchy of items for the custom field of type
hierarchy.initializegenerate_rootinsert_itemWhat approach did you choose and why?
Merge checklist