Hi Ron
I didn't find any other issue on this problem.
We have a big problem on biblio contributors, with the fusion & retain as alternate form.

We have 3 authors :
- cid 1375 : David Canevet (the good cid)
- cid 1862 : Canevet, David (we will do an alternate form of this one)
- cid 1597 : Canevet, D. (this one doesn't interest us)

On 1375, we chose "fusion + retain as alternate form" for 1862 author. See snapshot 1.
As you can see on snapshot 2, 1862 author is now linked to 1375.
You can verify it in the database, cf. snapshot 3.

And when you go on the third contributor, 1597, ... surprise, fusion & retain checkboxes are checked, cf. snapshot 4 - even if, as you can see in the database, this is not true.

So this relation is not real, but...
case 1 : if you uncheck the box, then the relation between the 2 others contributors disappears.
case 2 : you do not change anything and save the record, the relation becomes real, in place of the first one which disappear.

I hope you understand this description, ask me for more details.
Thank you for helping us with this problem.

CommentFileSizeAuthor
#5 biblio-2242501-5-hacky-workaround.patch2.99 KBAlan D.
#5 Screenshot.png41.21 KBAlan D.
snapshot4.png48.46 KBSt_B
snapshot3.png21.79 KBSt_B
snapshot2.png47.83 KBSt_B
snapshot1.png50.45 KBSt_B
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

BaptisteJ’s picture

BaptisteJ CreditAttribution: BaptisteJ commented

Hi,

In biblio/includes/biblio.admin.inc file line 2201, perhaps it should not be :

$merge_def = ($other_author->alt_form && $other_author->aka) ? $author->cid : FALSE;
But :
$merge_def = ($other_author->alt_form == $author->cid && $other_author->aka == $author->cid) ? $author->cid : FALSE;

I hope it will be helpful !

St_B’s picture

St_B CreditAttribution: St_B commented
Status: Active » Reviewed & tested by the community
St_B’s picture

St_B CreditAttribution: St_B commented

We found two new bugs.

Bug 2 : Relations broken
The first one is corrected by rewriting biblio_admin_author_edit_form_merge_link() in includes/biblio.admin.inc, line 2372 : 

      //->condition(db_or()->condition('cid', $cid)->condition('cid', $ccid))
      ->condition('cid', $ccid)->condition('aka', $cid)
      ->execute();

Without this correction, this happens :
- author A is merged (with retain as alternate form) to author B
- on author C, in the merge_def appear authors A and D. Checkboxes are emtpy. We choose to merge C and D - checkbox of author A stays empty - and save => C and D are well merged, but the relation between A and B is broken. It seems that the line of author A is reseted because of the checkbox being on 0 position.

Bug 3 : Fusion and alternate forms, rebounds

We have nid 1, with cid A as contributor ; nid 2 with cid B as contributor. biblio_contributor =

nid cid merge_cid
1 A 0
2 B 0

We merge A and B, retaining B as alternate form.
biblio_contributor_data =

cid aka alt_form
A
B A A

And now biblio_contributor =

nid cid merge_cid
1 A 0
2 B A

If you rebound on biblio?f[author]=A, you have exactly the same results as if you rebound on biblio?f[author]=B. Curiously this rebound uses biblio_contributor, not biblio_contributor_data ; both cid and merge_cid in biblio_contributor.

As B is an alternate form, we can select it again in a node 3. This is the result :

biblio_contributor =

nid cid merge_cid
1 A 0
2 B A
3 B 0

PROBLEM : now when you rebound on biblio?f[author]=B, you have one node more than on biblio?f[author]=A, because merge_cid is empty.

I think that what is done in nodes when you fusion two authors should be done each time you save a node :
If the field alt_form of the contributor selected is not empty, then the value of alt_form goes in biblio_contributor.cid, and the "rejected" form goes in merge_cid. This would be logic (it would be a real fusion with alternate form, and not only the same thing as a link), and would resolve the rebounds problem.

I also think the rebounds should be rewrited in order to use biblio_contributor_data instead of biblio_contributor, don't you think so ?

St_B’s picture

St_B CreditAttribution: St_B commented
Status: Reviewed & tested by the community » Needs work
Alan D.’s picture

Alan D. CreditAttribution: Alan D. commented
Status: Needs work » Active
FileSize
Screenshot.png41.21 KB
biblio-2242501-5-hacky-workaround.patch2.99 KB

We just hit this today too. It's this logic in the submit handler that assumes unchecked should be blanked out.. Similar for merge too.

/admin/config/content/biblio/author/CID/edit

    if ($options['link']) {
.....
    }
    else {
      // This is blanking out unrelated relationships
      db_update('biblio_contributor_data')
      ->fields(array(
          'aka' => 0,
          'alt_form' => 0,
      ))
      ->condition(db_or()->condition('cid', $cid)->condition('cid', $ccid))
      ->execute();
    }

I've attached a quick workaround / hack that prevents the error, but unlikely to be the best way. If it finds an author that is used elsewhere, to doesn't add it to the form, rather to makes a note and shows links to these other authors.

Set issue status to active as I don't think this patch is the right approach, rather it's a hacky workaround to stop data loss

wmfinnegan’s picture

wmfinnegan CreditAttribution: wmfinnegan commented

The workaround seems to solve the problem for me (which I'd only noticed after cleaning up a few hundred authors). Would another approach be to remove authors from the merge/link table (just like you can add authors via the autocomplete below the table)?

AndrewGearhart’s picture

AndrewGearhart CreditAttribution: AndrewGearhart commented

I think that's probably a strong approach. The fuzzy logic that is applied to get the multiple authors in the table is good, however, as mentioned, the unchecked authors (if linked between other authors) are removed.
If you could control which authors are in the table, ... that would be one way of handling it. Although, removing 6 authors every time could get cumbersome.

Another option would be to determine a way to prevent the update of unchecked options unless a checkbox on the form says to do so. Thoughts?