Request Plugin needs some changes

[xinu]

22:48:53 - xinu: a missing directive in org.drftpd.commands.config.hooks
22:49:08 - xinu: for request
22:49:12 - xinu: request directive handler
22:49:33 - Dom: not sure we need that
22:50:49 - xinu: yep
22:50:52 - xinu: because of
22:50:55 - xinu: src/plugins/org.drftpd.commands.request/resources/conf/ftpcommands.conf.dist
22:51:04 - xinu: request.dirpath /requests
22:51:13 - xinu: is only added for "SITE REQUESTS"
22:51:49 - Dom: well we add it for the others there then
22:51:55 - xinu: so if you dont specify a request.dirpath the plugin tries to use the cwd
22:53:38 - xinu: mh, thats twice then.... if we specify the "request.dirpath" everywhere we dont need to check the cwd for the request permission...
22:53:53 - Dom: I know
22:54:05 - Dom: the request command code is A) old, B) unfinished
22:54:10 - Dom: much like lots of it
22:55:30 - Dom: only reason for a directive would be for using multiple request paths
22:55:41 - Dom: but then site requestS etc can't really deal with that anyway
22:56:06 - Dom: and irc !request would have some issues too ;)
22:57:29 - xinu: mh right
22:57:51 - xinu: but adding the "request.dirpath" more then one time for every command doesnt make sense for the most
22:58:03 - xinu: having a config file like conf/plugins/request.conf
22:58:10 - Dom: yeah was going to say that
22:58:11 - xinu: would be better i guess
22:58:22 - Dom: request code was last touched before conf/plugins existed though ;)
22:58:30 - xinu: ok
22:58:43 - xinu: i just wanted to say you what i found out today :)
22:58:54 - Dom: should ticket them though
22:58:57 - Dom: or I'll forget
22:59:03 - xinu: im just beta tester
22:59:12 - xinu: ill make a ticket
22:59:24 - xinu: with this irc log
22:59:25 - xinu:

<DONE>

[fr0w]

I'm looking foward making some improvements on the request plugin.

[erich]

I've just tested [2010] and SITE REQUEST <something>, comes with 530.

I have checked perms.conf with different settings. It does not seem to work!

[erich]

Above mentioned error is caused by a missing request.dirpath in 3 of the command sections within ftpcommands.conf.dist for the request plugin.

Diff attached here: http://drftpd.org/forums/viewtopic.php?f=19&t=3305&p=14592#p14592

[fr0w]

What do you think it's best: add a directive to perms.conf or create a plugins/requests.conf?

perms.conf

request /REQUESTS/ =request !*

plugins/requests.conf

request.path=/REQUESTS/
request.perms=-johndoe =siteop =request
 
reqfilled.prefix=FILLED-
request.prefix=REQUEST-
...

requests.conf is more flexible, but perms.conf is simpler.

What do you think?

[SundaY]

Not sure you need to add any perms setting at all, you set the permissions in ftpcommands.conf and irccommands.conf

something like this would be prefered:

plugins/requests.conf

# Request path
request.dirpath=/requests

reqfilled.prefix=FILLED-
request.prefix=REQUEST-

# Max allowed requests/week
request.weekmax=2
# Users exempt from the weekmax rule
request.weekexempt=

Also the irccommands should be moved from org.drftpd.plugins.sitebot to request plugin.

[SundaY]

Another thing, "deleteOthers =siteop" is added to ftpcommands.conf but not irccommands.conf for REQDEL. If we keep it in command files and not store it in request.conf it needs to be added to irccommands.conf as well.

[djb61]

I think the original reason for having a request.dirpath in the commands.conf was to allow for more than one requests dir. For example you could have "!request" going to a normal dir and allowed for all users and then say a "!viprequest" with restricted access going to another dir. If say a siteop wanted to purge normal user requests frequently but leave those requested by certain people for longer.

I'm not sure if this is needed or not, but we could simply say if request.dirpath is set in the command then use that, otherwise fallback to using a request.conf. This would allow more complex setups whilst being transparent to a user wanting the simple use case. Or maybe something whereby multiple paths could be config'd in request.conf with some simple type reference from the commands.conf, again falling back to a default to make it transparent in the simple case.

I don't think we need to have the request pathperm in perms.conf, I only commented the code so as to make it partially work but didn't delete it initially in case someone else knew of a reason for keeping it that I've overlooked. I think we can obtain all the user controls we need simply by using the permissions for the commands themselves.

[fr0w]

Most of the work was commited in [2014] but there is some minor things that still needs to be done.

1. What's the proper way to notify the events generated by the request plugin? IRC Announcers or Events?
2. PostHook to increment REQUESTSFILLED
3. PostHook to increment REQUESTS
4. Implement Weekly allotment for requests (using a pre/post hooks)
5. Decrease the usage of the weekly allotment when reqdel (using a post hook, optional)
6. Sync Request.irc.properties
7. Move the declaration of the requests irc commands from org.drftpd.plugins.sitebot 'irccommands.conf' to this plugin (as requested by Scitz0)

Dom, what do you think about 1?

I'll be addressing 2, 3, 4 soon.

What about 5, what should we do when a release is deleted (through site reqdelete)

Scitz0 or erich, can any of you work on 6 & 7 as I'm not a SiteBot user?

[scitz0]

1) the gain with irc announcer instead of just returning the success message with the response for irc are that you can control were you want the output to go instead of just the channel you sent the command from. Also if done through ftp you can still get the message on irc. I can fix this, need to suppress the success response to irc, either by setting the success value in the properties empty or do as I did in Nuke.java

if (request.getSession() instanceof BaseFtpConnection) {
	response.addComment(request.getSession().jprintf(_bundle, _keyPrefix+"nuke", env, requestUser));
	response.addComment(nukeeOutput);
}

Ie, add the success message to response if command was sent from ftp, else let the irc announcer handle it. Whats prefered?

2-4 sounds good, though out interest, why increment REQUESTS and REQUESTSFILLED with posthook and not in the command it self? I'm sure the reason is very good, I just want to learn more about dr/java design.

5 is not something I would use but I guess others might find it useful so feel free to implement, cant hurt as long as you have an option to turn it off.

6-7 I could take a look at, dom mentioned the reason for the settings in sitebot plugin though. If you dont build the sitebot you shouldn't get irccommands.conf.dist files with commands you cant execute anyway, atleast for core plugins.

[scitz0]

All items fixed in [2035]