As discussed in #1173814: Replace php target selection with something better there is a very strong argument for voting target to be an instance specific setting.

CommentFileSizeAuthor
#5 0002-Removing-trailing-whitespace-and-updating-update-fun.patch2.19 KBericduran
#2 0003-Issue-1205166-Field-settings-should-be-an-instance-s.patch12.19 KBjames.elliott
#1 0001-Issue-1205166-by-james.elliott-Changed-Voting-target.patch6.3 KBjames.elliott

Comments

james.elliott’s picture

james.elliott commented
Status: Active » Needs review
StatusFileSize
new 0001-Issue-1205166-by-james.elliott-Changed-Voting-target.patch6.3 KB

And here is the patch!

Changes include:

  • Moves the axis setting from fivestar_field_settings_form to fivestar_field_instance_settings_form
  • Altered _fivestar_field_target() so that it will get the target from the instance of the field
  • Altered fivestar_get_tags() so that it returns an associative array in order to properly set the axis for fields
james.elliott’s picture

james.elliott commented
Title: Voting target should be an instance specific setting » Field settings should be instance specific
StatusFileSize
new 0003-Issue-1205166-Field-settings-should-be-an-instance-s.patch12.19 KB

I think the only setting that makes sense to keep as a field level setting is voting tag.

Attached patch moves all the settings to instance settings.

ericduran’s picture

ericduran commented
Status: Needs review » Needs work

This needs work.

The issue seems to be field never validates, This is because of the patch I pushed yesterday, either I missing one in between, or there's something missing on this patch.

ericduran’s picture

ericduran commented
Priority: Normal » Critical
ericduran’s picture

ericduran commented
StatusFileSize
new 0002-Removing-trailing-whitespace-and-updating-update-fun.patch2.19 KB

Here's a patch on top of this one to clean up the whitespace, and changes the update hook to be 7202 instead of 7002.

Also this patch needs to be applied on top of James patch. I'm doing it this way so everyone can keep they're proper commit attributions :)

ericduran’s picture

ericduran commented

Issue mention in #3 still applies FYI.

ericduran’s picture

ericduran commented
Status: Needs work » Fixed

This is now fixed. I had to add a couple of patches in between to fix the broken schema issue. I applied the other patches on top of this one, so James can get proper commit credit :) (/me wishes every contrib module worked like this)

--
http://drupalcode.org/project/fivestar.git/commit/a5a9f79c0103f1809d97f0...
http://drupalcode.org/project/fivestar.git/commit/a95fd5a5389c47ab43a915...
http://drupalcode.org/project/fivestar.git/commit/df44598399289afb4c8ad8...
http://drupalcode.org/project/fivestar.git/commit/2d88a9a9e55acc0c0116ce...

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.