Needs work
Project:
Signup
Version:
5.x-2.x-dev
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
3 Sep 2007 at 04:41 UTC
Updated:
4 Feb 2013 at 13:06 UTC
Jump to comment: Most recent file
Based on the TODO list at http://groups.drupal.org/node/5044
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | signup_rolespecific.patch.txt | 13.76 KB | tatien |
| #8 | signup_rolespecific.module.patch_2.txt | 12.16 KB | tatien |
| #7 | signup_rolespecific.module.patch_1.txt | 12.16 KB | tatien |
| #6 | signup_rolespecific.module.patch_0.txt | 12.69 KB | tatien |
| #5 | signup_rolespecific.install.patch_0.txt | 902 bytes | tatien |
Comments
Comment #1
tatien commentedI had made a patch for this, I really thought I had posted it :(. Now I can't even find it on my computer.
I will get back to you ASAP. The patch was installed on a prod. website, I just need to get it back.
Comment #2
tatien commentedPatched. Here is the patch for the .module file.
Comment #3
tatien commented... and here the patch for the .install file.
Comment #4
dwwyour patch appears to be against revision 1.107 of signup.module. many things have changed since then. ;) if you actually look at the patch, you'll see a huge # of lines of code that are modified, most of which are just undoing recent important changes to the module. please apply your changes to the latest version in HEAD (revision 1.116) and make a new patch that only includes your changes. thanks.
Comment #5
tatien commentedAll right, I have re-implemented the patches on the 1.120 version. Here is the first part (install).
Comment #6
tatien commented... and here the .module patch.
Comment #7
tatien commentedSorry, I had forgot to remove some debugging code in the last .module patch. Please ignore it and use this one instead. The .install file was ok.
Comment #8
tatien commentedAgain, some fixes to the patch. Please ignore the others.
Comment #9
dwwA) 4 out of 20 hunks FAILED -- saving rejects to file signup.module.rej.
B) Code style: always use {} after your if () clauses, even if it's just a single line.
C) This comment doesn't make much sense:
// Set default type access to default general access if nothing was specified for this content type yet.D) Code style:
'signup_access_' . $type-- no space between the.and string literals.E) In this code block, if $node isn't set, trying to assign something to
$node->typeis an error. Also, see point (B) above:F) Code style: always include a comma at the end of the last thing you declare in an array, for example:
should be:
G) Can you explain why the patch is doing the following?
H) Seems like this access stuff should be exported to views somehow. Like, you want to filter signup nodes based on if the current user is allowed to signup for them, etc.
I) signup_handler_filter_role() seems like duplicate code with core's user_roles() function.
Comment #10
dwwp.s. that said, this seems like it could be a useful feature when it is done. thanks for the patches so far! ;)
Comment #11
dwwComment #12
tatien commentedHi, I hereby address each of your points.
A) The CVS version probably changed since the time I submitted the patch.
B) OK.
C) Ok, I removed it.
D) Ok.
E) Ok.
F) Ok.
G) Because we need default options in that case (and NULL as the param of _signup_admin_form() returns default values).
H) Yes, that would be great!
I) Allright, I will change it.
Now, the problem is that I don't want to redo the work too often, so we'd need to coordinate a bit cause if I make a patch today for the current CVS checkout and we wait too long, it won't be valid anymore and we'll still have the (A) problem. So, I don't know what is the best way to do it, what do you think?
Thanks.
Comment #13
Totakeke commentedHas there been any updates on this feature request? It's something that would be very useful but it seems progress has been stopped dead in its tracks.
Comment #14
dwwSadly, I can't promise I always have time to review a new patch as soon as it's rolled. Problem (A) is basically just a fact of life in Drupalandia, sorry. :( You can watch the issue queue to see if there's something else actively being rerolled that might conflict. Now that this is running on g.d.o again, I might have to make some changes based on feedback from that experience, but I don't personally have a ton of time to work on signup for the next week or so, anyway. That said, the problems usually aren't too major and re-rolling is often a quick and painless task.
@tatien: All I can say is that if you roll a new patch that addresses all known concerns, I'll review it and either explain why it needs work or commit it as soon as I can. Feel free to assign this to yourself if you're going to actively work on the patch.
Comment #15
tatien commentedThanks. Here is a new patch on the CVS.
Comment #16
juliekj commentedIs there anything on this patch for the 6.x release?
Comment #17
Apollo610 commentedA D6 version would be fantastic... this sounds an awful lot like the Signups by Role module that's out there (also only in D5 form right now). But it would be great if Signups had its own permissioning built in.
Though that maybe a bit difficult considering the number (and scope) of outstanding requests there are out there for this high-demand mod.
Comment #18
duaelfrThis version of Signup is not supported anymore. The issue is closed for this reason.
Please upgrade to a supported version and feel free to reopen the issue on the new version if applicable.
This issue has been automagically closed by a script.
Comment #19
simon georges commentedReverting recent closing.