Closed (fixed)
Project:
Privatemsg
Version:
6.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
23 Dec 2008 at 01:37 UTC
Updated:
5 Mar 2009 at 21:50 UTC
Jump to comment: Most recent file
Comments
Comment #1
litwol commentedlets do it :)
Comment #2
naheemsays commented?
the files just need to be removed from cvs.
Comment #3
litwol commentedi meant to say we should move contents of privatemsgapi.inc into privatemsg.module
Comment #4
naheemsays commentedWhy? we only moved them the other way recently.
They can move there, but is there a reason for this? The need for no longer having an includes? for the api functions?
IMO we should go the other way and the api file should be merged back with the privatemsg.module, and admin, pages functions split out as per drupal 6 conventions.
Comment #5
litwol commentedwha ? i thought that is what i said..
Comment #6
naheemsays commentedoops - I read it all wrong. sorry. :)
Comment #7
naheemsays commentedThis causes the site to gie a white page of death for me, but I cannot tell why...
It:
1. removes the privatemsgapi file
2. adds a privatemsg.admin.inc file
3. adds a privatemsg.pages.inc file
4. Does not work.
Comment #8
berdirThe first problem I had was a privatemsg_setup_includes call that was still there on line 285. I got a white page after that too but after cleaning the cache (with devel), it does work fine for me.
There is probably even more that can be moved to privatemsg.pages.inc, for example the form processing functions like pm_send.
I had some small issues when I applied the patch (especially with privatemsgapi/privatemsgapi.info, which was not deleted for some reason) but apart from that, it does work fine.
Comment #9
naheemsays commentedI left the pm_send function there for now as the api functions that replacing them would probably be in the main module file?
Comment #10
naheemsays commentedremoved the privatemsg_setup_includes() call mentioned above and also moved the theme preprocess functions back into the module page - this fixes again the viewing of messages. Also moved pm_cancel into privatemsg.pages.inc.
Comment #11
naheemsays commentedthe extra files have been deleted, but removing the api file is still wanted IMO.
Comment #12
litwol commentedooo i'm afraid to commit this patch for now. it will break too many other patches. can we split this up into multiple smaller patches? first moving api.inc into privatemsg.module and then the rest ?
Comment #13
naheemsays commentedThe above patches can be disregarded - hence set to back to "active" instead of "Patch cnw/cnr". That work will have to be done again much later on.
It can be done in multiple steps, but I would prefer the other patches to go in first before doing this - same reason as you. too disruptive.
Comment #14
berdirSplitting up in multiple patches sounds good for me, I would propose something like the following steps:
1) move/delete functions in privatemsgapi.inc to privatemsg.module, without any code-changes
2) create privatemsg.pages.inc and move possible functions to that file
3) Not so important/Optional: clean up function prefixes (change pm_ and private_message_ to privatemsg_).
Patches that change privatemsgapi.inc and should go in first are (from what I remember):
#288183: Provide api function for other modules to send messages.
#371861: Flush deleted messages
Comment #15
berdirI need to update #371861: Flush deleted messages anyway, so here is a patch for step #1
Changes:
- deleted those save_group functions and moved _privatemsg_assemble_query and privatemsg_load to privatemsg.module
- removed privatemsg_setup_includes, calls to that function and privatemsg_init()
Comment #16
litwol commentedRIP privatemsgapi.inc
I fixed E_NOTICE along with this patch for message_actions.