Code review comment for lp:~rvb/maas/cluster-own-ui4

Revision history for this message
Raphaël Badin (rvb) wrote :

> 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.

« Back to merge proposal