> Wonderful! And of course we've gone over the design in depth already.
Thanks a lot for the review!.
> [...]
>
> Also in get_view_title, line 401 of the diff:
>
> title_chunk = '<a href=%s>%s</a>' % (link, title_chunk)
>
> Shouldn't the value for the href attribute be quoted? This may be a
> consequence of the method being slightly too big for comfortable detailed
> testing.
That's correct, I'm splitting that method into smaller bit now.
> I'm not too comfortable about the bulk accept/reject buttons applying to all
> pending clusters... somebody could attempt to register a malicious cluster
> after the admin loaded the page, but before they hit the “accept all” button.
> Or conversely, though less harmfully I guess, a good cluster could present
> itself after the admin loaded the page but just before they hit the “reject
> all” button.
It's a bit dangerous indeed. This is not something this branch changed though. Let's fix this later.
> Wonderful! And of course we've gone over the design in depth already.
Thanks a lot for the review!.
> [...]
>
> Also in get_view_title, line 401 of the diff:
>
> title_chunk = '<a href=%s>%s</a>' % (link, title_chunk)
>
> Shouldn't the value for the href attribute be quoted? This may be a
> consequence of the method being slightly too big for comfortable detailed
> testing.
That's correct, I'm splitting that method into smaller bit now.
> I'm not too comfortable about the bulk accept/reject buttons applying to all
> pending clusters... somebody could attempt to register a malicious cluster
> after the admin loaded the page, but before they hit the “accept all” button.
> Or conversely, though less harmfully I guess, a good cluster could present
> itself after the admin loaded the page but just before they hit the “reject
> all” button.
It's a bit dangerous indeed. This is not something this branch changed though. Let's fix this later.
> [...]
All your other remarks: done.
Thanks again.