Summary

Fix 500 errors from upstream files not existing

Review Request #8591 — Created Jan. 4, 2017 and submitted Jan. 12, 2017, 2:12 p.m.

Information

Owner

brennie

Repository

Review Board

Branch

release-2.5.x

Bugs

Depends On

Commit

88b9deb...

Reviewers

Groups

reviewboard

People

Description

Previously, trying to download a original or modified file based on an

upstream file that does not exist would result in a 500 error. Now we

check for that case and, if the file does not exist, we return a 404

error instead. Unit tests have been added to ensure this behaviour does

not regress.
Testing done:

Ran unit tests.

Testing Done

Ran unit tests.

Issues

Description	From	Last Updated
Rather than implementing this here, let's just add a get_file and get_file_exists to TestService in reviewboard/testing/hosting_services.py that returns this if …	chipx86	Jan. 4, 2017, 1:30 p.m.
We should probably log the failure.	david	Jan. 5, 2017, 1 p.m.

Tool: Pyflakes
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/reviews/tests/test_views.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/reviews/tests/test_views.py

```
Thanks for taking care of this :)
```

reviewboard/reviews/tests/test_views.py (Diff revision 1)

The issue has been resolved. Show all issues

Rather than implementing this here, let's just add a get_file and get_file_exists to TestService in reviewboard/testing/hosting_services.py that returns this if given, say, /invalid-path as the path. Then this logic will be reusable and this unit test becomes smaller.

Commit:

ded15c5668c79eca673761c566ff56559dfa235b

543cfac0ff34429dc7a6d738fb9e39a5bf24fd35

Diff:

Revision 2 (+136 -1)

Show changes

	reviewboard/extensions/tests.py
	reviewboard/reviews/views.py
	reviewboard/reviews/tests/test_views.py

Tool: Pyflakes
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/reviews/tests/test_views.py
    reviewboard/extensions/tests.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/reviews/tests/test_views.py
    reviewboard/extensions/tests.py

reviewboard/reviews/views.py (Diff revision 2)
The issue has been resolved. Show all issues
```
We should probably log the failure.
```

Commit:

543cfac0ff34429dc7a6d738fb9e39a5bf24fd35

88b9deb0a6049a5c4e0b76d393d13de905fa5514

Diff:

Revision 3 (+139 -1)

Show changes

	reviewboard/extensions/tests.py
	reviewboard/reviews/views.py
	reviewboard/reviews/tests/test_views.py

Tool: Pyflakes
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/reviews/tests/test_views.py
    reviewboard/extensions/tests.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/reviews/tests/test_views.py
    reviewboard/extensions/tests.py

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-2.5.x (1f34ec8)