upenndigitalscholarship / deep

MIT License
3 stars 1 forks source link

Black Letter filter not working quite right #171

Closed ZacharyLesser closed 8 months ago

ZacharyLesser commented 1 year ago

There are a number of records where the data seems fine but they are appearing in the YES filter rather than the NO filter. But there is nothing in the field "Black Letter" at the Edition level. Nothing in that field should return NO, and does for other records:

These DEEP #s are not filtering correctly: 15, 5024, 5024.01, 181, 203, 146, 182, 277, 335, 336, 183, 491, 337, 523, 586, 338, 365, 524, 5077.03, 339, 5090.02, 340, 549, 341, 368, 551, 932, 935, 936, 1009, 5143.05, 5143.06, 5143.12, 5143.16, 5143.32, 1088, 1092, 1117

There may be others. The underlying question is why is the filter working most of the time but not all of the time?

Black Letter (Edition Level) -- any data in field = YES no data in field = NO

apjanco commented 10 months ago

From the db, there was '', None, "Yes", "Yes, partly", "Yes" and "No" Changed to explicit conditions for these four cases, resolving to either yes or no in item_data. Now returning 210 with Black Letter "Yes", which the same as db.

ZacharyLesser commented 10 months ago

It looks like the original data, including "Yes, partly", got erased in this fix. We want that data to display to users, since there is a difference to them between "Yes" and "Yes, partly." But both should hit the yes filter.

ZacharyLesser commented 10 months ago

The options in the "Blackletter" data field (Edition Level) should only be:

Yes Yes, partly [no data in field]

Anything like "none" must have been added automatically during the import.

The filter should return yes if it says either Yes or Yes, partly. It should return no if there is no data in field.

ZacharyLesser commented 10 months ago

The reason to do it this way is that if there is no data in that field, then in the results display the entire header "Black Letter" should not be shown to the user. It only gets displayed in the case of Yes or Yes, partly

ZacharyLesser commented 10 months ago

The filter is functioning correctly -- the titles returned for both Yes and No match the old site as they should.

Display of results is not quite right because when data has "Yes, Partly", the display of the record to the user still just says "Yes" -- it should display what is in the Blackletter data field. (This also enables additional functionality in case we later want to add other descriptive info to the data field, such as "Yes, but on title page only" or something -- filter will still function properly and we can make this kind of change without touching the code.)

ZacharyLesser commented 8 months ago

"Yes, Partly" now displays properly for those records (see DEEP 5009, or DEEP 51, e.g.)

BUT: these are not appearing on the results list for Black Letter = Yes

Black Letter = Yes should include all items with anything in the Black Letter field in the db. This includes "Yes", "Yes, Partly," and anything else we might want to put in that field for display purposes in the future (such as "Yes, on title page only").

The logic is:

Filter on Black Letter: any data in field = "Yes"; no data in field = "No"

Display: if data in field, display the line "Black Letter: [data in field]"