Item status -- Barcodes with initial or trailing spaces fail silently when in txt file

Attaching file without spaces

Marking this confirmed. We've had reports of failed file uploads, and this could be a contributing factor as I was able to duplicate on my side.

If the file contains a mismatch of barcodes with and without spaces at the end, the barcodes without the spaces will still load.

This happens also in 3.1.5.

Still in recent-ish master; noting that blank hard-return lines at the end of the barcode list will also cause a failure.

That's https://bugs.launchpad.net/evergreen/+bug/1813954

Ah, many thanks Elaine. LP search foils me yet again! I've confirmed bug 1813954.

I often have more luck searching in Google rather than LP. For this one, I just pulled up bugs I reported

It looks like for some reason, the regex that strips trailing spaces and commas, is also stripping off \r (carriage returns), but only for the barcodes that don't have trailing spaces..

Ohhh, it is that $ match. The $ is matching the \r, but if there are spaces or commas, it matches those and leaves the \r.

line = line.replace(/(.*?)($|\s.*|,.*)/,'$1');

Here is the barcode array before and after the regex.

Pre import barcodes:
Array(17) [ "35500004207930,\r", "\r", "33500012542726, \r", "35500004468045,\r", "\r", "33500012225165\r", "35500005727696 \r", "35500006028581\r", "35500004587208 \r", "35500003474093\r", … ]

Imported barcodes:
Array(12) [ "35500004207930\r", "33500012542726\r", "35500004468045\r", "33500012225165", "35500005727696\r", "35500006028581", "35500004587208\r", "35500003474093", "35500004587182\r", "35500006007197", … ]
app.js:300:13

I wonder of the original author of this was using files created in Linux, with no carriage returns. I just converted my file to unix EOL, and now it works fine as written. I think the code needs to handle both.

Josh

Looks like the simple fix is for the split to look for both CRLF and LF.

angular.forEach(newVal.split(/\r?\n/)

That solves the issues with spaces and newlines for me.

I'll create a branch.

Josh

Branch at user/stompro/lp1798187-item-status-barcode-import-crlf
https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/stompro/lp1798187-item-status-barcode-import-crlf

I also noticed that when non of the barcodes in the file match, there was an error in the console about the code trying to select the imported barcodes. I added an extra check to prevent that from coming up.

Josh

Tested in sandbox.

I created a file of barcodes from the concerto database and added trailing and initial spaces.

Trailing spaces were ignored but a barcode with initial spaces halted the import

My file: (␢ represents a blank space)

CONC40000570␢␢
CONC41000570
CONC4000070
CONC4100070
␢␢CONC4200070
CONC4300070
CONC4400070
CONC50000670
CONC51000670
CONC50000170
CONC40000569
CONC41000569
CONC4000069
CONC4100069
CONC4200069
CONC4300069
CONC4400069
CONC50000669
CONC51000669
␢␢CONC50000169
CONC40000630
CONC41000630
CONC40000130
CONC41000130
CONC42000130
CONC43000130
CONC44000130␢␢
CONC50000730
CONC51000730
CONC50000230

When the file loaded, only the last 10 displayed:

CONC40000630
CONC41000630
CONC40000130
CONC41000130
CONC42000130
CONC43000130
CONC44000130␢␢
CONC50000730
CONC51000730
CONC50000230

Created a file with blank lines at end (using above barcodes) and the file loaded with no problems

CONC40000570
CONC41000570
CONC4000070
CONC4100070
CONC4200070
CONC4300070
CONC4400070
CONC50000670
CONC51000670
CONC50000170
CONC40000569
CONC41000569
CONC4000069
CONC4100069
CONC4200069
CONC4300069
CONC4400069
CONC50000669
CONC51000669
CONC50000169
CONC40000630
CONC41000630
CONC40000130
CONC41000130
CONC42000130
CONC43000130
CONC44000130
CONC50000730
CONC51000730
CONC50000230
[blank line]
[blank line]

Also used same barcodes to create a file with a blank line in the middlish. Imported the file with no problems.

Just to be thorough, I also created a file with blank lines at the beginning. The file imported fine.

I was looking at how the xul client handled the barcode import, and it handled it using a single split statement.

https://git.evergreen-ils.org/?p=Evergreen.git;a=blob;f=Open-ILS/xul/staff_client/server/circ/copy_status.js;hb=HEAD#l470

var barcodes = content.split(/[,\s]+/);

\s matches:
    A space character
    A tab character
    A carriage return character
    A new line character
    A vertical tab character
    A form feed character

so any combination of whitespace and commas would work in the xul client.
29393999393, 2992929299299, 29939393939,,,, 3939399939

Maybe we should just go back to that? Would there ever be a need to allow spaces in the middle of barcodes?

I think the current webclient code could handle something like
1234 567 2344
8888 888 8888

But if we just want to go back to the XUL client behavior then using the same split regex should work.

Otherwise, the webclient code could strip leading spaces also.
Josh

I don't think there would be many recent barcode formats that would have spaces that are not stripped out when scanned. The barcode on the label might be 5 07230 1013677 4 but scans as 50723010136774.

I vote for using the XUL method since we did not have the issues with it we are seeing with the web client.

Elaine

+1 for the XUL regular expression.

FWIW, we had a library in our consortium that did have spaces in the barcodes themselves (not just the label, but the actual barcode content). These barcodes were handled so inconsistently in various Evergreen interfaces, that they finally gave up and rebarcoded all of those items to a format without spaces. I think we should document the standards and assumptions we have about barcodes somewhere, so that libraries new to Evergreen or thinking about their barcode schema can know what to expect. If others agree, I can open a new launchpad bug for that.

I think documenting the barcodes standards for both items and patrons is a good idea.

New bug for the documentation piece here: https://bugs.launchpad.net/evergreen/+bug/1818692

I'm in favor of documenting standards for barcodes in Evergreen.

I'm also in favor of stripping all the characters Josh mentions in comment #14 as in the xul method.

However, the xul client never handled files of barcodes with spaces between the barcode characters well. When uploading, the xul client would see:

1234 567 2344

as three distinct barcodes:

1234
567
2344

I am also seeing problems uploading a file of barcodes with spaces between characters in the current web client. It fails silently.

So in addition to handling trailing and leading characters in the uploaded file, the web client should handle spaces between characters in barcodes better than it does now.

This may not be practical, but I'll throw it out here. Ideally I would like to see all whitespace characters stripped from barcodes before saving to the database and all whitespace characters stripped from barcodes entered into the client. If others agree, I'll gladly open a separate bug.

I agree with having white space characters stripped from barcodes before saving and entering.

The XUL client took any whitespace/newline/comma characters to be separators between barcodes.

So
28383883883, 28282828288282, 2828282828828828, 92993939939393
and
28383883883 28282828288282 2828282828828828 92993939939393
and
28383883883
     28282828288282
  2828282828828828
    92993939939393
and
28383883883
28282828288282
2828282828828828
92993939939393

all had the same result. which is why spaces inside barcodes would cause the behavior described.

So is that the wanted behavior?

Or do we want to say that each barcode must be on a separate line, and strip all whitespace?

So
38838384848484848, 282828838383 ,28288882822
would be read as one barcode.
3883838484848484828282883838328288882822
so we wouldn't support csv format.

28828 282828 8282882
200303 030303 30030
200200 200200 20020
would be
288282828288282882
20030303030330030
20020020020020020

Josh

My vote would be for each barcode on a separate line, strip all whitespace.

Updated branch at user/stompro/lp1798187-item-status-barcode-import-crlf
https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/stompro/lp1798187-item-status-barcode-import-crlf

Each barcode on a separate line, all whitespace/commas removed from barcodes.

Also includes a commented out regex for only stripping trailing/leading whitespace/commas just in case a site does use barcodes with internal spaces.

Josh

It may also be useful to have a pop-out help link right next to the file upload, so a user can just click the "?" and get a quick description of the file format required. That is faster than trying to find the information in the docs.

Maybe a tooltip, although the amount of info may be too much for a tooltip.
https://material.angular.io/components/tooltip/overview
Here is an example of a pop open tooltip that stays open, something like this may work.
https://ej2.syncfusion.com/angular/demos/#/material/tooltip/html-content

I'm also curious if the file upload should report back on the barcodes that failed to load? I don't think the per failure popup like the XUL client is all that user friendly, but maybe just a report at the end of how many barcodes couldn't be loaded, and which ones couldn't be loaded? Maybe it could just display as part of the progress monitor, and if there are failures the progress monitor would stay open until dismissed?

Josh

+1 to having a pop-out help. You should be able to add one with the following code in the tt2 file:

 <eg-help-popover help-text="[% l('Barcodes should not contain spaces') %]">

And you can replace the text within l('') with the guidance you'd like to give to the user.

Also, +1 to reporting back on which barcodes failed to load, and for it to be less intrusive than the XUL popups.

+1 to reporting back on the failed barcodes, and less intrusive than XUL.

+1 to reporting back on the failed barcodes, and less intrusive than XUL.

I force updated my last branch with the help popup, thanks Jane. I couldn't position it right where I wanted it because the input button for the file upload is wide to show filenames, but I think it works fine where it is.

Popup message is "File Format: One barcode per line. All whitespace and commas will be removed before processing."

I have a working example of alerting about barcodes that were not loaded. I should probably create a new bug for that. Or should I just add it to this bug to reduce overhead for testing?
Josh

I'm fine with keeping the alert as part of this bug, but would not mind if others determined it was best to separate it.

I've tested and signed off on the current patch:

working/user/gmcharlt/lp1798187_item_status_barcode_import_signoff
https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/gmcharlt/lp1798187_item_status_barcode_import_signoff

I suggest opening a separate bug for the alerting, as the patch as is provides a good improvement and should probably go in sooner rather than later.

this an issue at Albany Public Library.
Lynn

Thanks, all! I've pushed this to master, rel_3_3, rel_3_2, and rel_3_1 -- this will be included in the 3.4, 3.3.3, 3.2.8, and 3.1.14 releases.

Josh: I'm interested in the alert you've come up with. Does your alert also catch deleted items? If so, maybe we can continue the notification part of this discussion over at https://bugs.launchpad.net/evergreen/+bug/1739288