Empty librarian tags started showing up

[mietcls]

Bug description

Empty librarian tags started showing up. Edit 23 January 2024: still there.

On Test, Dev and Production:

Steps to Reproduce

1. Go to the publications overview on https://backoffice.bibliotest.ugent.be/publication
2. Click on the librarian tags filter and see the empty status
3. Try filtering on the empty status, and see it not work. No console log.

I cannot seem to reproduce records that are added to that filter.

[nicolasfranck]

In development found these publications:

01GK16JB2ERX1BJV09YC34X2EG 8767782 8769171

When I looked into their values there where empty values added in the list

e.g. for 01GK16JB2ERX1BJV09YC34X2EG they where:

"esci"
""
"funding to do: bof"
""

So I guess the reviewer tags should be cleaned on empty values in the list? I guess this may happen elsewhere too

Fixing this in the indexed document seems straightforward, by simply "compacting" the values. Or do this before storing the record in the database? Only applying this in the index hides ugly values that are present in the database, and we have to remember to keep doing these fixes when indexing these records.

Update: for some reason the flag "vacuum" does not seems to be working when processing the incoming form For example, when you enter

"a"
""
"b"
""

The vacuum filter will delete the parameters with no values:

"reviewer_tags[0]": []string{"a"}
"reviewer_tags[2]":[]string{"b"}

but that probably leaves a "hole" (at index 1)

From the documentation of https://github.com/go-playground/form:

Slice honours the specified index. eg. if "Slice[2]" is the only Slice value passed down, it will be put at index 2; if slice isn't big enough it will be expanded.

but else where it states Supports both Numbered and Normal arrays eg. "Array[0]" and just "Array" with multiple values passed.. This means that we do not have to add these square brackets to make arrays work. I tested this behaviour by manually editing the "name" attribute, and changing them everywhere to "reviewer_tags" (so without square brackets) and that seems to be true.

So, do we really need these square brackets? I mean, passing reviewer_tags=tag1&reviewer_tags=tag2 seems to work also again the current backend server code. That would remove the need for things like data-tmpl-name and the live renaming of that name by javascript, and the need for the correct index values.

The only reason why these square brackets with indexes would be necessary, would be when you pass an array of objects, each with their own attributes (e.g. person[0][name]=Franck&person[0][first_name]=Nicolas&person[1][name]=Steenlant ..), but do we have these kind of complex forms?

[nics]

@nicolasfranck the vacuum filter should remove these, i'll look into why not our code should work, independent of how the parameters are formatted

[nicolasfranck]

If we remove the brackets and the indexes from the names, we need to check the following:

• does every browser send the input fields in the same order as they were in the HTML document?
• are the form elements decoded in the same order server side? This is especially important for fields with the same name.

[nicolasfranck]

@nics I just remembered that we are going to return to the "all in one page" forms" (as was in LibreCat) in the future. That will require structured field names like abstract[0]["text"] and abstract[1]["text"] in order to know what value belongs to what element in the struct(ure). Removing the brackets would make it impossible to assign structured data server side, right?

[nics]

@nicolasfranck that doesn't matter for a simple array of text values, we can mix the 2 styles

[nics]

@mietcls this can be closed?

[mietcls]

The behaviour still shows up unfortunately, both on dev and production:

dev

production

[nicolasfranck]

mm, apparently I only fixed the input from the website, and not the indexation of existing records.

Will look into it.

[nics]

@nicolasfranck you can add it to the CleanupPublications (and Datasets) grpc method (see keyword cleanup) it the input is already being sanitized, they should disappear after cleanup+reindex