From 5dd3a4c8bcd0429f91b133bb0f0f22ee54192d04 Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Mon, 4 Nov 2013 10:00:37 +0100 Subject: [PATCH] wip: sort spam messages by score. Code improvement. see #375 --- modoboa/extensions/amavis/lib.py | 30 ++++++++++ modoboa/extensions/amavis/views.py | 87 +++++++---------------------- modoboa/extensions/webmail/lib.py | 14 ++++- modoboa/extensions/webmail/views.py | 68 ++++------------------ modoboa/lib/webutils.py | 45 +++++++++++++++ 5 files changed, 121 insertions(+), 123 deletions(-) diff --git a/modoboa/extensions/amavis/lib.py b/modoboa/extensions/amavis/lib.py index 2a98f4ccf..266503351 100644 --- a/modoboa/extensions/amavis/lib.py +++ b/modoboa/extensions/amavis/lib.py @@ -8,6 +8,7 @@ from django.core.urlresolvers import reverse from django.utils.translation import ugettext as _ from modoboa.lib import parameters from modoboa.lib.exceptions import ModoboaException +from modoboa.lib.webutils import NavigationParameters def selfservice(ssfunc=None): @@ -76,3 +77,32 @@ recipient=%s if re.search("250 [\d\.]+ Ok", answer): return True return False + + +class QuarantineNavigationParameters(NavigationParameters): + """ + Specific NavigationParameters subclass for the quarantine. + """ + def __init__(self, request): + super(QuarantineNavigationParameters, self).__init__( + request, 'quarantine_navparams' + ) + + def back_to_listing(self): + """Return the current listing URL. + + Looks into the user's session and the current request to build the + URL. + + :return: a string + """ + url = "listing" + params = [] + navparams = self.request.session[self.sessionkey] + if "page" in navparams: + params += ["page=%s" % navparams["page"]] + params += ["%s=%s" % (p, navparams[p]) + for p in ["criteria", "pattern"] if p in navparams] + if params: + url += "?%s" % ("&".join(params)) + return url diff --git a/modoboa/extensions/amavis/views.py b/modoboa/extensions/amavis/views.py index 3ac6f0840..7b136740c 100644 --- a/modoboa/extensions/amavis/views.py +++ b/modoboa/extensions/amavis/views.py @@ -19,32 +19,11 @@ from modoboa.extensions.admin.models import Mailbox, Domain from modoboa.extensions.amavis.templatetags.amavis_tags import ( quar_menu, viewm_menu ) -from .lib import selfservice, AMrelease +from .lib import selfservice, AMrelease, QuarantineNavigationParameters from .sql_listing import SQLlisting, SQLemail, get_wrapper from .models import Msgrcpt -def __back_to_listing(request): - """Return the current listing URL. - - Looks into the user's session and the current request to build the - URL. - - :param request: a ``Request`` object - :return: a string - """ - url = "listing" - params = [] - if "page" in request.session: - params += ["page=%s" % request.session["page"]] - - params += ["%s=%s" % (p, request.session[p]) - for p in ["criteria", "pattern"] if p in request.session] - if params: - url += "?%s" % ("&".join(params)) - return url - - def empty_quarantine(request): content = "
%s
" % _("Empty quarantine") ctx = getctx("ok", level=2, listing=content, paginbar="", @@ -52,37 +31,6 @@ def empty_quarantine(request): return HttpResponse(simplejson.dumps(ctx), mimetype="application/json") -def store_navparams(request): - """Store navigation paramters into session. - - :param request: current request - """ - import re - - sessionkey = 'quarantine_navparams' - if not sessionkey in request.session: - request.session[sessionkey] = {} - navparams = request.session[sessionkey] - navparams["order"] = request.GET.get("sort_order", "-date") - navparams["page"] = int(request.GET.get("page", 1)) - for param, defvalue in [('pattern', ''), ('criteria', 'from_addr')]: - navparams[param] = re.escape(request.GET.get(param, defvalue)) - - -def get_navparam(request, param, defaultvalue=None): - """Retrieve a navigation parameter. - - Just a simple getter to avoid using the full key name to access a - parameter. - - :param request: current request object - :param str param: parameter name - :param defaultvalue: default value if none is found - :return: parameter's value - """ - return request.session['quarantine_navparams'].get(param, defaultvalue) - - @login_required def _listing(request): flt = None @@ -93,26 +41,27 @@ def _listing(request): if not Domain.objects.get_for_admin(request.user).count(): return empty_quarantine(request) - store_navparams(request) - pattern = get_navparam(request, 'pattern', '') + navparams = QuarantineNavigationParameters(request) + navparams.store() + pattern = navparams.get('pattern', '') if pattern: - criteria = get_navparam(request, 'criteria') + criteria = navparams.get('criteria') if criteria == "both": criteria = "from_addr,subject,to" for c in criteria.split(","): if c == "from_addr": - nfilter = Q(mail__from_addr__contains=request.session["pattern"]) + nfilter = Q(mail__from_addr__contains=pattern) elif c == "subject": - nfilter = Q(mail__subject__contains=request.session["pattern"]) + nfilter = Q(mail__subject__contains=pattern) elif c == "to": - rcptfilter = request.session["pattern"] + rcptfilter = pattern continue else: raise Exception("unsupported search criteria %s" % c) flt = nfilter if flt is None else flt | nfilter msgs = get_wrapper().get_mails(request, rcptfilter) - page = get_navparam(request, 'page') + page = navparams.get('page') lst = SQLlisting( request.user, msgs, flt, navparams=request.session["quarantine_navparams"], @@ -255,8 +204,10 @@ def delete(request, mail_id): message = ungettext("%(count)d message deleted successfully", "%(count)d messages deleted successfully", len(mail_id)) % {"count": len(mail_id)} - return ajax_response(request, respmsg=message, - url=__back_to_listing(request)) + return ajax_response( + request, respmsg=message, + url=QuarantineNavigationParameters(request).back_to_listing() + ) def release_selfservice(request, mail_id): @@ -303,8 +254,10 @@ def release(request, mail_id): message = ungettext("%(count)d request sent", "%(count)d requests sent", len(mail_id)) % {"count": len(mail_id)} - return ajax_response(request, "ok", respmsg=message, - url=__back_to_listing(request)) + return ajax_response( + request, "ok", respmsg=message, + url=QuarantineNavigationParameters(request).back_to_listing() + ) else: msgrcpts = [] wrapper = get_wrapper() @@ -329,8 +282,10 @@ def release(request, mail_id): len(mail_id)) % {"count": len(mail_id)} else: message = error - return ajax_response(request, "ko" if error else "ok", respmsg=message, - url=__back_to_listing(request)) + return ajax_response( + request, "ko" if error else "ok", respmsg=message, + url=QuarantineNavigationParameters(request).back_to_listing() + ) @login_required diff --git a/modoboa/extensions/webmail/lib.py b/modoboa/extensions/webmail/lib.py index 0155968ee..05dc1c1d7 100644 --- a/modoboa/extensions/webmail/lib.py +++ b/modoboa/extensions/webmail/lib.py @@ -10,7 +10,7 @@ from django.core.files.uploadhandler import FileUploadHandler, SkipFile from django.utils.translation import ugettext as _, ugettext_lazy from django.conf import settings from modoboa.lib import u2u_decode, tables, parameters -from modoboa.lib.webutils import size2integer +from modoboa.lib.webutils import size2integer, NavigationParameters from modoboa.lib.email_listing import EmailListing from modoboa.lib.emailutils import ( EmailAddress, Email, prepare_addresses, set_email_headers @@ -745,3 +745,15 @@ class AttachmentUploadHandler(FileUploadHandler): def file_complete(self, file_size): return None + + +class WebmailNavigationParameters(NavigationParameters): + """ + Specific NavigationParameters subclass for the webmail. + """ + def __init__(self, request, defmailbox=None): + super(WebmailNavigationParameters, self).__init__( + request, 'webmail_navparams' + ) + if defmailbox is not None: + self.parameters += [('mbox', defmailbox)] diff --git a/modoboa/extensions/webmail/views.py b/modoboa/extensions/webmail/views.py index 1248b4e5a..2ecc7bfaf 100644 --- a/modoboa/extensions/webmail/views.py +++ b/modoboa/extensions/webmail/views.py @@ -22,7 +22,7 @@ from .lib import ( decode_payload, AttachmentUploadHandler, save_attachment, ImapListing, EmailSignature, clean_attachments, set_compose_session, send_mail, - ImapEmail + ImapEmail, WebmailNavigationParameters ) from templatetags import webmail_tags @@ -63,8 +63,9 @@ def move(request): if not arg in request.GET: raise WebmailError(_("Invalid request")) mbc = get_imapconnector(request) - mbc.move(request.GET["msgset"], get_navparam(request, 'mbox'), request.GET["to"]) - resp = listmailbox(request, get_navparam(request, 'mbox'), update_session=False) + navparams = WebmailNavigationParameters(request) + mbc.move(request.GET["msgset"], navparams.get('mbox'), request.GET["to"]) + resp = listmailbox(request, navparams.get('mbox'), update_session=False) resp.update(status="ok") return ajax_simple_response(resp) @@ -191,7 +192,7 @@ def editfolder(request, tplname="webmail/folder.html"): res["newmb"] = form.cleaned_data["name"] res["oldparent"] = oldparent res["newparent"] = pf - remove_navparam(request, 'mbox') + WebmailNavigationParameters(request).remove('mbox') return ajax_simple_response(res) ctx["mboxes"] = mbc.getmboxes(request.user) @@ -219,7 +220,7 @@ def delfolder(request): mbc = IMAPconnector(user=request.user.username, password=request.session["password"]) mbc.delete_folder(name) - remove_navparam(request, 'mbox') + WebmailNavigationParameters(request).remove('mbox') return ajax_response(request) @@ -301,7 +302,7 @@ def render_mboxes_list(request, imapc): :param imapc: an ``IMAPconnector` object :return: a string """ - curmbox = get_navparam(request, "mbox", "INBOX") + curmbox = WebmailNavigationParameters(request).get("mbox", "INBOX") return _render_to_string(request, "webmail/folders.html", { "selected": curmbox, "mboxes": imapc.getmboxes(request.user), @@ -309,52 +310,6 @@ def render_mboxes_list(request, imapc): }) -def store_navparams(request, extra_parameters=None): - """Store navigation parameters into session - - :param request: current request object - """ - import re - - sessionkey = 'webmail_navparams' - if not sessionkey in request.session: - request.session[sessionkey] = {} - navparams = request.session[sessionkey] - navparams["page"] = int(request.GET.get("page", 1)) - navparams["order"] = request.GET.get("sort_order", "-date") - for param, defvalue in [("pattern", ""), ("criteria", "from_addr")]: - navparams[param] = re.escape(request.GET.get(param, defvalue)) - if extra_parameters is None: - return - for param, defvalue in extra_parameters: - navparams[param] = re.escape(request.GET.get(param, defvalue)) - - -def get_navparam(request, param, defaultvalue=None): - """Retrieve a navigation parameter. - - Just a simple getter to avoid using the full key name to access a - parameter. - - :param request: current request object - :param str param: parameter name - :param defaultvalue: default value if none is found - :return: parameter's value - """ - return request.session['webmail_navparams'].get(param, defaultvalue) - - -def remove_navparam(request, param): - """Remove a navigation parameter from session. - - :param request: current request object - :param str param: parameter name - """ - navparams = request.session["webmail_navparams"] - if param in navparams: - del navparams[param] - - def listmailbox(request, defmailbox="INBOX", update_session=True): """Mailbox content listing @@ -366,9 +321,10 @@ def listmailbox(request, defmailbox="INBOX", update_session=True): :param defmailbox: the default mailbox (when not present inside request arguments) :return: a dictionnary """ + navparams = WebmailNavigationParameters(request, defmailbox) if update_session: - store_navparams(request, [('mbox', defmailbox)]) - mbox = get_navparam(request, 'mbox') + navparams.store() + mbox = navparams.get('mbox') lst = ImapListing( request.user, request.session["password"], baseurl="?action=listmailbox&mbox=%s&" % mbox, @@ -376,7 +332,7 @@ def listmailbox(request, defmailbox="INBOX", update_session=True): elems_per_page=int(parameters.get_user(request.user, "MESSAGES_PER_PAGE")), **request.session["webmail_navparams"] ) - return lst.render(request, get_navparam(request, 'page')) + return lst.render(request, navparams.get('page')) def render_compose(request, form, posturl, email=None, insert_signature=False): @@ -553,7 +509,7 @@ def index(request): raise WebmailError(_("Bad request")) response = dict(selection="webmail") - curmbox = get_navparam(request, "mbox", "INBOX") + curmbox = WebmailNavigationParameters(request).get("mbox", "INBOX") if not request.is_ajax(): request.session["lastaction"] = None imapc = get_imapconnector(request) diff --git a/modoboa/lib/webutils.py b/modoboa/lib/webutils.py index aa47ea23e..2d9f410a1 100644 --- a/modoboa/lib/webutils.py +++ b/modoboa/lib/webutils.py @@ -162,3 +162,48 @@ def topredirection(request): path = "userprefs" return HttpResponseRedirect(path) + + +class NavigationParameters(object): + """ + Just a simple object to manipulate navigation parameters. + """ + + def __init__(self, request, sessionkey): + self.request = request + self.sessionkey = sessionkey + self.parameters = [('pattern', ''), ('criteria', 'from_addr')] + + def store(self): + """Store navigation parameters into session. + """ + if not self.sessionkey in self.request.session: + self.request.session[self.sessionkey] = {} + navparams = self.request.session[self.sessionkey] + navparams["order"] = self.request.GET.get("sort_order", "-date") + navparams["page"] = int(self.request.GET.get("page", 1)) + for param, defvalue in self.parameters: + navparams[param] = re.escape(self.request.GET.get(param, defvalue)) + + def get(self, param, default_value=None): + """Retrieve a navigation parameter. + + Just a simple getter to avoid using the full key name to + access a parameter. + + :param str param: parameter name + :param defaultvalue: default value if none is found + :return: parameter's value + """ + if not self.sessionkey in self.request.session: + return default_value + return self.request.session[self.sessionkey].get(param, default_value) + + def remove(self, param): + """Remove a navigation parameter from session. + + :param str param: parameter name + """ + navparams = self.request.session[self.sessionkey] + if param in navparams: + del navparams[param]