Merge lp://staging/~danilo/loggerhead/bug-839395 into lp://staging/loggerhead

Proposed by Данило Шеган
Status: Merged
Merged at revision: 456
Proposed branch: lp://staging/~danilo/loggerhead/bug-839395
Merge into: lp://staging/loggerhead
Diff against target: 45 lines (+20/-4)
2 files modified
loggerhead/controllers/revision_ui.py (+8/-4)
loggerhead/tests/test_controllers.py (+12/-0)
To merge this branch: bzr merge lp://staging/~danilo/loggerhead/bug-839395
Reviewer Review Type Date Requested Status
John A Meinel Approve
Review via email: mp+73766@code.staging.launchpad.net

Description of the change

= Bug 839395 =

Make sure hacking a URL to go to a per-revision log for a certain file that is not part of that revision change causes no OOPS.

I am sure the test could be better, but it at least fails where expected, and with the fix it starts passing.

Also, maybe it'd be better to throw a 404 when the path is not matched, so any suggestions on how do we do that (and write a test for it) are welcome.

To post a comment you must log in.
Revision history for this message
John A Meinel (jameinel) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 09/02/2011 11:49 AM, Данило Шеган wrote:
> The proposal to merge lp:~danilo/loggerhead/bug-839395 into lp:loggerhead has been updated.
>
> Description changed to:
>
> = Bug 839395 =
>
> Make sure hacking a URL to go to a per-revision log for a certain file that is not part of that revision change causes no OOPS.
>
> I am sure the test could be better, but it at least fails where expected, and with the fix it starts passing.
>
> Also, maybe it'd be better to throw a 404 when the path is not matched, so any suggestions on how do we do that (and write a test for it) are welcome.
>
> For more details, see:
> https://code.launchpad.net/~danilo/loggerhead/bug-839395/+merge/73766

You can raise an HTTPException from the paste family to get the web ui
to give a 404.

See "inventory_ui.py" line 113:

        try:
            revid = self.get_revid()
            rev_tree = branch.repository.revision_tree(revid)
        except errors.NoSuchRevision:
            raise HTTPNotFound()

So probably you could do:

if len(items) == 0:
  raise HTTPNotFound()
...

If you want to do it, great. If not, we can still merge what you've done.
  merge:approve

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk5gqR4ACgkQJdeBCYSNAAPv3ACdHiO8tLNMw143aVotg93oYglz
qAEAoLpXbMtSAe7fNtFpI0bwwPWR34Lh
=eXdO
-----END PGP SIGNATURE-----

Revision history for this message
John A Meinel (jameinel) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches