Arianek and I decided to split up [#632716]into smaller issues/patches. Please review the relevant section of the last patch for this that can be found at http://drupal.org/node/632716#comment-2285200

Post review / screenshots and rerolled patches here -- thanks!

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

arianek’s picture

Status: Active » Needs review
FileSize
3.6 KB

manually cut this from the mondo patch

tobiasb’s picture

Status: Needs review » Reviewed & tested by the community
lisarex’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
40.17 KB
2.38 KB

This patch hasn't been reviewed properly yet.

- put caps on modules
- moved link to handbooks as per other help patches
- put caps on linked pages

I'm sure there's more to say about this module, but I don't know it well enough :)
Also, the text is word-for-word what's in the handbook page. Should we try to get more inventive (if there's time?)

Status: Needs review » Needs work

The last submitted patch failed testing.

lisarex’s picture

FileSize
40.17 KB

Please review the patch that passed and ignore the failed patch. It was actually a png with a bad file extension :D

arianek’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
3.02 KB

assuming the functionality of this module hasn't changed massively, this looks good to me - made one small grammar tweak, but otherwise thumbs up.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Cool, committed to HEAD.

This help file probably needs a good revamp with some more step-by-step instructions, but I'm not sure who other than jhodgdon, cwgordon7, or jvandyk would be able to write it. If such a thing were to materialize in the next week or so, though, I'd happily commit it.

jhodgdon’s picture

Assigned: Unassigned » jhodgdon

I'll take a stab at it, as soon as I wake up. :)

jhodgdon’s picture

Status: Fixed » Needs work
jhodgdon’s picture

Status: Needs work » Needs review
FileSize
7.56 KB
28.83 KB
14.09 KB
21.89 KB

OK. Here's an update. I changed the help for the Trigger module and on the Trigger and Actions admin pages. Also changed some header text on the Actions page. See screenshots.

As a note, I think that one of the most confusing things people might not realize is that they have to make an advanced action when it might seem like they need an advanced trigger. For instance, you want a certain action to take place only if certain words are in the content. But you can't trigger on "content is created with certain words" (triggers are not configurable). You instead trigger on 'content is created' and the action is "unpublish content with certain words'. I tried to make this clear in the help text, without calling attention to the weirdness of it all. Hopefully. Other than that, I think the steps are pretty clear (verify/configure the action and then assign it to a trigger), so I didn't think too much needed to be said.

arianek’s picture

- on the Triggers page i think it should be "...such as new content being added OR a user logging in"
- on the Actions page, i think it should be "Create AN advanced action" for the title

(didn't roll a patch as the second is debatable)

jhodgdon’s picture

Agree with your first suggestion (typo). Second is OK too.

jhodgdon’s picture

FileSize
7.57 KB

Here's a patch incorporating both of the suggestions from #11. No other changes, and I didn't make new screen shots.

arianek’s picture

Status: Needs review » Reviewed & tested by the community

getting

Hunk #1 succeeded at 2789 (offset 9 lines).
Hunk #2 succeeded at 2816 (offset 9 lines).

but it seems to apply regardless - RTBC

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Great. Thanks. Committed to CVS HEAD.

Status: Fixed » Closed (fixed)
Issue tags: -d7help

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