Skip to content
This repository has been archived by the owner on Jan 27, 2025. It is now read-only.

Make the full record available in binds #66

Closed
fsteeg opened this issue Nov 8, 2021 · 10 comments · Fixed by #83
Closed

Make the full record available in binds #66

fsteeg opened this issue Nov 8, 2021 · 10 comments · Fixed by #83

Comments

@fsteeg
Copy link
Member

fsteeg commented Nov 8, 2021

See #60 (review).

Functional review: @TobiasNx
Code review: @blackwinter

@blackwinter
Copy link
Member

Copying previous discussion for better overview:


This prevents access to the actual record. Is this intentional?

Originally posted by @blackwinter in #60 (comment)


My idea was that in the scope of the bind, the record (as the subexpressions see it) should only contain the bound value. I'm not sure why though, that might very well make no sense.

Originally posted by @fsteeg in #60 (comment)


I wasn't able to find it documented anywhere, but Catmandu seems to preserve access to the whole record:

$fixes = <<EOF;
do list(path:foo,var:loop)
  copy_field(test,loop.baz)
end
EOF

$fixer = Catmandu::Fix->new(fixes => [$fixes]);

is_deeply $fixer->fix({foo => [{bar => 1}, {bar => 2}], test => 42}),
    {foo => [{bar => 1, baz => 42}, {bar => 2, baz => 42}], test => 42},
    'binding scope testing';

Originally posted by @blackwinter in #60 (comment)


I agree, the full record should be available. I've opened #66 since this does require some work that I probably won't have time to do this week (simply using the full record with the current implementation results in overwriting previous values under the same key, which is covered by a test).

Originally posted by @fsteeg in #60 (comment)

@blackwinter
Copy link
Member

Catmandu's behaviour seems to depend on the presence of the var option:

$fixes = <<EOF;
do list(path:foo)
  copy_field(test,baz)
end
EOF

$fixer = Catmandu::Fix->new(fixes => [$fixes]);

is_deeply $fixer->fix({foo => [{bar => 1}, {bar => 2}], test => 42}),
    {foo => [{bar => 1}, {bar => 2}], test => 42},
    'binding scope w/o var testing';

See also hbz/Catmandu@22d7359.

@fsteeg
Copy link
Member Author

fsteeg commented Dec 1, 2021

Deployed to test: access outer type from inside the bind (see issue on prod).

@TobiasNx
Copy link
Collaborator

TobiasNx commented Dec 1, 2021

Deployed to test: access outer type from inside the bind (see issue on prod).

The test-example uses a do list-bind with an non list field is this right?

Catmandu does only iterate with an do list only over lists:

# List over all items in demo and add a foo => bar field
# { demo => [{},{},{}] } => { demo => [{foo=>bar},{foo=>bar},{foo=>bar}]}
do list(path: demo)
  add_field(foo,bar)
end

Source: http://librecat.org/Catmandu/#fixes-cheat-sheet

@TobiasNx TobiasNx assigned fsteeg and unassigned TobiasNx and fsteeg Dec 1, 2021
@TobiasNx
Copy link
Collaborator

TobiasNx commented Dec 1, 2021

The example should be something like that:
See here

It seems that arrays from formeta are not processed correctly.

@TobiasNx
Copy link
Collaborator

TobiasNx commented Dec 1, 2021

PS:
I have to correct myself. They are processed correctly since resource is procesed as a list but has no array sign.

But it shows that the handling of repeated fields as arrays coming out of fix is not understood properly by the encoders yet. If turning it into xml it looks like this: here

It also shows that in FIX fields with the same name are overwritten and the last instance of this fields is passed on. In our case since "title[]" is not created as array but as simple field only the last instance title[]": "Book: Fausto is taken up.

@fsteeg
Copy link
Member Author

fsteeg commented Dec 1, 2021

To stay focused on the scope issue, here is an example with repeated fields:

See solution on test, compare issue on prod.

(The issues you mention seem to be related to #80 and #82.)

@fsteeg fsteeg assigned TobiasNx and unassigned fsteeg Dec 1, 2021
@TobiasNx
Copy link
Collaborator

TobiasNx commented Dec 1, 2021

+1 seems to work.

PS: Should I open another ticket concerning that non array fields should not work with list-binds?

@fsteeg
Copy link
Member Author

fsteeg commented Dec 1, 2021

Should I open another ticket concerning that non array fields should not work with list-binds?

I think we should have a uniform way to process fields that may or may not be repeated. How do you iterate over the values of a field that may or may not be repeated in Catmandu?

@TobiasNx
Copy link
Collaborator

TobiasNx commented Dec 1, 2021

Should I open another ticket concerning that non array fields should not work with list-binds?

I think we should have a uniform way to process fields that may or may not be repeated. How do you iterate over the values of a field that may or may not be repeated in Catmandu?

I think there is no such function in catmandu but we could ask somebody of the Catmandu-Team. Or open a ticket at their repo.

@TobiasNx TobiasNx assigned fsteeg and unassigned TobiasNx Dec 1, 2021
@fsteeg fsteeg removed their assignment Dec 1, 2021
@fsteeg fsteeg removed their assignment Dec 1, 2021
fsteeg added a commit that referenced this issue Dec 3, 2021
@fsteeg fsteeg closed this as completed in #83 Dec 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants