Skip to content

models: HostBib: Properly handle 774s without $w#53

Open
awilfox wants to merge 1 commit into
mainfrom
awilfox/AP-778-fix-nil-handling
Open

models: HostBib: Properly handle 774s without $w#53
awilfox wants to merge 1 commit into
mainfrom
awilfox/AP-778-fix-nil-handling

Conversation

@awilfox

@awilfox awilfox commented Jul 14, 2026

Copy link
Copy Markdown
Member

Using .empty? still requires the value to not be nil. There is at least one record that has no w and therefore has a nil value; using .present? checks for nil and empty values.

Fixes: LIBIT-10644
Ref: AP-778

@anarchivist anarchivist left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r+; looks like a reasonable change. do we need another spec/fixture to handle testing 774s without subfield $w?

the rubocop failure appears unrelated.

@awilfox awilfox force-pushed the awilfox/AP-778-fix-nil-handling branch from 239aef5 to 1d52b43 Compare July 14, 2026 21:47

@davezuckerman davezuckerman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than the rubocop issue it looks good to go.

@awilfox

awilfox commented Jul 14, 2026

Copy link
Copy Markdown
Member Author

v2:

  • Fix rubocop issue (prefer present over blank).

@awilfox

awilfox commented Jul 14, 2026

Copy link
Copy Markdown
Member Author

r+; looks like a reasonable change. do we need another spec/fixture to handle testing 774s without subfield $w?

Yeah, I think that's a good idea. I'll add one.

@awilfox

awilfox commented Jul 14, 2026

Copy link
Copy Markdown
Member Author

Since this should be processed like the other records, I'm not sure where to add it in spec/lib/bibliographic/report_spec.rb; would it be mmsid_succeeded_without_774w and no line in the log content?

Using `.empty?` still requires the value to not be `nil`.  There is
at least one record that has no `w` and therefore has a `nil` value;
using `.present?` checks for nil *and* empty values.

Fixes: LIBIT-10644
Ref: AP-778
@awilfox awilfox force-pushed the awilfox/AP-778-fix-nil-handling branch from 1d52b43 to e30346e Compare July 14, 2026 22:40
@awilfox

awilfox commented Jul 14, 2026

Copy link
Copy Markdown
Member Author

v3:

  • Fix inverted logic caused by trying to fix the Rubocop issue, which thankfully triggered another Rubocop issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants