[newmaint-site] Clearing data for a data source / contribution type

[Marco Bardelli]

Hi list,
enrico at enricozini.org writes:

> On Tue, Apr 01, 2014 at 03:07:56AM +0200, Marco Bardelli wrote:
>
>> I propose this patch:
>>  http://anonscm.debian.org/gitweb/?p=users/safanaj-guest/dc.git;a=commitdiff;h=ad44f44606a6a9cbc2bf01277770b5686c397733
>> i left commented out some "excessive stuff" in get_context_data that
>> enumerate contributions during confirmation step.
I reviewed the stuff, look at 05cd0854491fe96908e3e9e69a79af89c4ca6cd8.
Between the two commits there is a third easily rebasable to fix a
typos, i think.

>
> Looks good. I didn't notice that SourceMixin and ContributionTypeMixin
> could be used like that, I like the idea a lot.
If we want to enter deeply in the "Class-based view tunnel", in sources
app, a lot of stuff could be restructured to maximize reuse, ie
permission checks, but obscuring a lot of simple code, i think
to *_{view,list} could use a Mixin to dispatch ("check perms") and a
get_context_data to fill response, and convert function in class with
some attributes to customize response. I don't believe it's will be
really useful, but only a stylistic change, "more deep in the tunnel".

> I think you can leave out the "excessive stuff": this is going to be
> mostly useful during early tries at data sources, to remove wrong data
> that hs been posted, and one is probably not so interested in a summary
> of the wrong data.
Left out.

> The view misses an admin check. Something like:
>
>         if not self.object.can_admin(self.request.user):
> 		raise http.Http404
>
> you can add a can_admin proxy method to ContributionType delegating to
> self.source.can_admin, to make it work in both cases.
I think this would be redundant, because already checked in parents Mixins,
ContributionTypeMixin.dispatch and SourceMixin.get_object.
This still true if ContributionsDeleteView is only used in that context.
I documented a lot this point. Tests still missing :(

I have some problem to play locally, adding data stuff, probably
README.md needs update about ./dc-post-source vs dc-tool from
python-debiancontributors package. Also in "unused ?" admin.py stuff
still some old code ? ie: UserProfile vs AbstractUserModel.

> I don't like the Http404, I wish django had a raisable Http403 as well.
Django, from 1.4, provides django.core.exceptions.PermissionDenied for
this purpose, here:
https://docs.djangoproject.com/en/1.5/topics/http/views/#the-403-http-forbidden-view
I used it for "can_admin" failure.

[...snip...]
>
> Ciao,
>
> Enrico
Ciao,

Marco
-- 
GPG key: 4096R/0xF7FE4DAA 2013-08-02 Marco Bardelli <bardelli.marco at gmail.com>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/newmaint-site/attachments/20140401/dd25166b/attachment.sig>