config.marc21_ff_pos_map needs an audit

Bug #1371647 reported by Jason Boyer
14
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Evergreen
Fix Released
Wishlist
Unassigned

Bug Description

Effects all versions, seed data specific.

This was brought to my attention by a cataloger using some of the advanced search options. When limiting audience to adolescents large print serials are returned in the results. Looking into this, this is because config.marc21_ff_pos_map defines a position for Audn in SER records. Serial records don't have an audience code, that position (008/22) is the Orig code, for original format (and the code for adolescent audiences is the same as the format code for large print).

I'm no cataloger, so while I can fix this particular issue locally, the other fixed field definitions need to be checked so any necessary changes can be made all at once. (Especially because I'm assuming that a reingest is required to completely correct the issue.)

Tags: pullrequest
Revision history for this message
Kathy Lussier (klussier) wrote :

Another example of this problem is that the literary form fields used for books are not used for other item types, like audiobooks. So whenever you do an audiobook search limited to a particular literary form, you will retrieve zero results.

Revision history for this message
Jason Boyer (jboyer) wrote :

Here they are: http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/collab/jboyer/lp1371647_more_fixed_fields The only field that needed to be removed was Audience on Serial records (which got this whole thing started) but I've also added all of the 006/008 fields that were not defined in config.marc21_ff_pos_map and added CCVM entries for those that are applicable. Depending on your desired use you might have to do more config (i.e. to use these fields as part of a composite ccvm definition, etc.)

tags: added: pullrequest
Revision history for this message
Jason Stephenson (jstephenson) wrote :

Hi,

This needs a release note to summarize the changes and indicate that a reingest is needed.

Also, the upgrade script should probably print the warning that a reingest is needed rather than just have a comment to that effect.

Finally, if you really want it in for 2.9-beta, target the milestone. I'm really only looking at milestone targeted bugs today.

Jason

tags: added: needsreleasenote
Jason Boyer (jboyer)
Changed in evergreen:
milestone: none → 2.9-beta
Revision history for this message
Jason Boyer (jboyer) wrote :

Thanks for the feedback but I've run into another stumbling block that's going to take longer to fix. :( Pushing this back to 2.next.

Changed in evergreen:
milestone: 2.9-beta → 2.next
Revision history for this message
Jason Boyer (jboyer) wrote :

DISREGARD PREVIOUS BRANCH!

Here is a proper branch that takes care of things and is even rebased to master (and the according shift in CCVM ids that came with it.)

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/jboyer/lp1371647_add_fixed_fields

tags: removed: needsreleasenote
Jason Boyer (jboyer)
Changed in evergreen:
milestone: 2.next → 2.9.1
Revision history for this message
Jason Stephenson (jstephenson) wrote :

I have a couple of questions:

Is this really a bug fix or is it a new feature? It seem like a new feature since things are being added that didn't exist before, unless of course the absence of those things is considered to have been a mistake/bug. I am leaning toward new feature, but I could be persuaded. Note that the bug is currently targeted as a bug fix. This is why I ask for opinions.

I see that the latest branch appears to take into account the changes in the 0934 upgrade script, but I wonder if there is any overlap between the two. I don't see any but the upgrade scripts are both rather large so it is hard to tell just from looking. Jason, can you verify that there is no overlap between the two upgrade scripts?

Jason Boyer (jboyer)
Changed in evergreen:
milestone: 2.9.1 → 2.next
Revision history for this message
Jason Boyer (jboyer) wrote :

Having taken a closer look at that, no, there's no possible way for them both to work as-is. I adjusted ids down based on the values in 950... but missed the addition of the entire 960... file. Away goes the pullrequest tag. :(

tags: removed: pullrequest
Revision history for this message
Jason Boyer (jboyer) wrote :

Crisis averted. Apparently the additions to CCVM from the tag tables service will always be inserted at ids above 10K*, while the additions of my patch don't go beyond 1734, leaving plenty of buffer. pullrequest is back!

*Neither 960.data.marc21-tag-tables.sql nor it's upgrade script specify ids, they just uses the same sequence that will have been bumped to 10K after id 711 is inserted in 950.data.seed-values.sql.

tags: added: pullrequest
Revision history for this message
Jason Boyer (jboyer) wrote :

Re: Jason's comment #6, I'm going to call this a feature rather than bug fix. The old data was "incomplete," but that's not really the same as "incorrect." (though 1 of them was. This is much, much larger than just fixing that one.)

Revision history for this message
Galen Charlton (gmc) wrote :

I'll review this during hackfest

Changed in evergreen:
assignee: nobody → Galen Charlton (gmc)
Kathy Lussier (klussier)
Changed in evergreen:
importance: Undecided → Wishlist
Revision history for this message
Galen Charlton (gmc) wrote :

Pushed to master for inclusion in 2.10, along with a tweak to resolve a conflict with ccvm IDs. Thanks, Jason!

Changed in evergreen:
milestone: 2.next → 2.10-beta
assignee: Galen Charlton (gmc) → nobody
status: New → Confirmed
status: Confirmed → Fix Committed
Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.