Basically I've removed hook_nodeapi in favor of hook_node_..., changed db_query to dbtng, and other fixes. The testcase pass completely, 0 minor warnings from coder.. I guess I did test all.
I'm missing a readme.txt file or other human readable reference, but my english is not as good as required to do so.
I've also fixed an error in the default value for the variable storing wheter the content type has the rating enabled. That value is TRUE on module's logic, and was 0 (FALSE) on content type settings.. Basically this means: rating was enabled by default in ALL content types, but administrator was not aware of when browsing the settings of the content types.
| Comment | File | Size | Author |
|---|---|---|---|
| #24 | examples.nodeapi_fixup_615466_24.patch | 4.92 KB | rfay |
| #19 | examples.nodeapi_example.patch | 18.85 KB | rfay |
| #4 | nodeapi_example_drupal_7_ilo_1.patch | 19.44 KB | ilo |
| nodeapi_example_drupal_7_ilo.patch | 18.92 KB | ilo |
Comments
Comment #1
ilo commentedI forgot to set needs review status.
Comment #2
jhodgdonRegarding "I've also fixed an error in the default value for the variable storing ..." -- do we need to go back and fix that in the Drupal 6 version as well?
And if I havent' already said so, THANKS for your work on this!!
Comment #3
ilo commentedI would say yes, it was original D6 code, and the error is there.
You are welcome ;)
Comment #4
ilo commentedrerolled to 7.x-1.x-dev
Comment #5
dave reidReminder: Please commit D7 patches to the HEAD CVS branch, not DRUPAL-7--1.
Comment #6
rfay@ilo, on #4, the patch *really* doesn't apply, even with -p3. Could you please reroll?
Thanks so much for your work on this.
Comment #7
rfayComment #8
ilo commentedrfay and I had a convo yesterday about the name of the module. in D6 hook_nodeapi gave the name to this module, but this hook no longer exists in D7. Should the module be renamed to node_api_example module? nodeapi_example may lead to confusion when you had previous experience using this hook.
Comment #9
dave reidI'd probably like to see this merged with node_example.module.
Comment #10
jhodgdonDave: The node example module and the node API example module actually serve two different purposes.
The node example module is an example of how to use a module to define a content type. The node API example is an example of how to use a module to add features to existing content types (such as Page, Story, Blog, and others defined by other modules).
The two modules use different sets of hooks, such as (in D7 terms) hook_delete() vs. hook_node_delete() -- the former being the hooks that are called only for the module that defined the content type, and the latter being the ones called via module_invoke() so they are invoked on any module that chooses to implement the hook.
So I do not think they should be merged.
+1 on the name change, though.
Comment #11
jhodgdonActually, I am not sure I'm in favor of the name change, after all. This will make it more difficult for someone looking at the 6.x version of the module on api.drupal.org to switch easily to the 7.x version and see what has changed. Of course, the function names for the hook implementations have changed anyway (because the hooks changed)...
Comment #12
jhodgdonAlso, if you decide to change the name, you need to file a patch on D7 core to update the references in function header docs to this example module.
Comment #13
ilo commentedThanks jhodgdon, this is actually what I was looking for, I had the feeling of some CONS to the name change that I'm still unable to realize. My actual concerns is that, well, nodeapi_example is not a bad name for a module explaining "node api" usage, even if "nodeapi" function does not exist in it, as long as email_example is not using a function called "email". The problem from 6.x to 7.x documentation at api.drupal.org would exist however, as some functions are new and others do not exist any more due to the api changes, but at least they have a file to look at. In fact there are some modules that will exist in D6 only and others in D7 only.
Should we consider the two targets in the file? I mean, IMO, we should focus on newcomers to Drupal 7 dev (had never heard about hook_nodeapi), as long as experienced Drupal 6 developers should be able to find the way to understand what happened to the nodeapi hook. I say this because in the D6 version, there is a lot of prosa about the diferences between 4.7, 5 and 6 versions of nodeapi, and all this text is going to disappear. I would not include a "what happened to hook_nodeapi and now there is a replacement" neither, but just document the set of node api hooks available, as if the D6 version of the module never existed.
That said, I'm NOT in favour of a name change, but if someone considers this as necessary I see no problems to do so.
Note: I did a search through the D7 installed and found no reference about nodeapi_example in any of the files, so I'm sorry, I don't know about what to patch..
Note 2: davereid, merging the two concepts will build up a huge monster module unable to understand for newcomers. It requires a great effort (think about a reason in the example, put the code in with a functionality) to explain the differences between hook_form_alter and hook_form_ID_alter, so now figure the results with 23 hook_node_whatever functions...
Comment #14
ilo commentedForgot to mention, I'd put an abstract about nodeapi_example module in D6 README.txt saying that this is a module illustrating how to work with "Node API functions" (currently hook_nodeapi only), to keep focus on the module usage instead of the module name = hook_nodeapi.
Comment #15
rfayMy opinion: Let's just keep the D7 version of this module as a demonstration of the various node hooks.
How about "node_hooks_example" as the new name of the D7 version?
Comment #16
rfayComment #17
tanoshimi commentedHi,
I downloaded the 7.x-dev release of "examples" module recently and, having looked through the out-of-date nodeapi example, I'd assumed that no work had been done to port to D7. Had I have found his thread earlier, maybe it would have saved me writing http://drupal.org/node/724588 - doh!
Anyway, for what it's worth, I would make the following comments:
- I would personally prefer the module to retain the "Node API" in its name, either as "nodeapi_example", or "node_api_example". Why? Firstly, for anyone who had previous experience of D6, this is the term they'll be looking for when upgrading their modules. Secondly, keeping this name emphasises the fact that this module recreates the same functionality as achieved by the nodeapi_example module in D6 (even if it does so with different hooks). And finally, even for those users who are coming to this module as fresh D7 developers, there is a huge amount of legacy documentation that refers to "node API" and I think it's a term that generally describes what this module does.
- I would like to see this patch rerolled/committed as soon as possible. "Examples for developers" is a really valuable module, and there are loads of people currently in the process of trying to upgrade their D6 modules to D7, scrabbling around to understand the myriad of changes and best practices that should be applied in conversion. When I downloaded nodeapi_module and found it was out-of-date, I just assumed that this module hadn't been revised yet and so went trawling through lots of threads on drupal.org to recreate the same result as ilo had already done, 4 months ago!
Anyway, great work everyone, and I look forward to more great examples in the future.
Comment #18
ilo commentedThe port begun several months ago, but as long as there are too many modules, not all have been upgraded yet. Should you reviewed the issue queue, maybe it would have thinking about the work already done ;)
The truth is that nodeapi hook DOES NOT exist anymore, and there is a new "Node API" function set.. it is not clear if the module's name came from the functionality or the hook itself, therefore needs some clarification. Removing nodeapi_example from D7 examples branch would notice developers that the hook no longer exists, and force them to look for the alternative API.
Althoug examples can be used to understand the process of module upgrading, it is not an oriented resource. It is focused on how to exploit all that core functionalities and how to write good code for them. The information about updating modules to D7 can be found here: Converting 6.x modules to 7.x
Fortunately all of us are interested in having a good documentation in the end, so any help would be appreciated!
Thanks for sharing your opinion on this, looks like this time the name of the module already matters.
Comment #19
rfayI'm sorry we got derailed on this so long ago. Ilo's work should have been committed.
D7 moved along a ways, but I was able to get this passing the tests and seeming to work.
I'm committing the attached to HEAD: http://drupal.org/cvs?commit=348470
We can go ahead and argue about how this should be presented. But in the meantime, the old not-updated example has been confusing people.
I do think we probably need some followup on this to make it easier to understand and use. But here it is working.
Comment #20
rfayWhoops! This completely prevents a node from being saved.
To test:
1. Try to edit a node.
2. Save it.
3. All changes lost.
Comment #21
rfayComment #22
rfayIf I just go to create a Page, and create one, I get:
and
and
However, if I first go and edit the content type and save it, everything is OK and it seems to work from that point onward.
Comment #23
rfayI created an example node (with node_example enabled) and on save got:
Comment #24
rfayThis fixup seems to resolve the nodeapi_example problems.
I fixed #801086: hook_theme no longer uses 'arguments' array key at the same time.
Comment #25
rfayCommitted: http://drupal.org/cvs?commit=369640