Hi,

Though I am a little confused about the approach of this module (pseudo field) it still does what I want it to do, so I go with it :-)

It seems that the current strategy for token replacement is a little odd. The tokens should not be replaced when the node is saved, but rather when it is rendered. Imagine the author name of a node should be part of a tweets text. When the author changes it's name without the node being re-saved, then the change of the user name will go undetected.

Also as mentioned in #1906932: Tokens not working for some reason and #1931640: Only uses value set in global tweet text or content type tweet text for newly created nodes this approach creates problems for node that have been created prior to installing this module. Instead, given the fact that this module provides a pseudo-field, that no user can ever edit using the UI, it seems only logical that the instance settings (as defined via the Fields UI) are used instead of the values actually stored.

I propose the attached patch that does the following:

  • Change default value logic, so that it escalates correctly: instance settings, if empty site-wide settings, if empty module defaults
  • Move token replacement from hook_field_presave() into the function tweetbutton_tweet_get_attributes().
  • Remove the implementation of hook_field_presave().
  • Button type selection: replaced switch construction with an array map.

At first I wanted to write this as a feature request, but it is more of an UI bug.

Comments

berliner’s picture

Status: Active » Needs review
berliner’s picture

StatusFileSize
new5.73 KB

Fixed a bug in the previous patch.

berliner’s picture

StatusFileSize
new5.54 KB

Corrected the patch again. I had mistakenly assumed, that there is no UI to change the fields settings (which seemed strange to me). I see now that this is not the case and therefore have changed the patch accordingly. It still corrects the default value chain and the token replacement logic.

berliner’s picture

Issue summary: View changes

corrected description

timmillwood’s picture

Status: Needs review » Closed (outdated)