Summary

Fix the "watched list" resource.

Review Request #14258 — Created Dec. 4, 2024 and submitted Dec. 17, 2024, 7:44 a.m.

Information

Owner

david

Repository

Review Board

Branch

release-7.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

This change fixes that up to use the correct method name, and
reimplements the body of it to skip the bulk of what the base get_list
implementation would have done.

Testing Done

Fetched the watched list resource and got a response with links instead
of an error.

Commits

Summary	ID
Fix the "watched list" resource. The watched list resource is a special resource that exists only to link to the other watched item resources. It was completely broken, returning HTTP 405, due to a bad implementation of our singleton resource pattern. This change fixes that up to use the correct method name, and reimplements the body of it to skip the bulk of what the base `get_list` implementation would have done. Testing Done: Fetched the watched list resource and got a response with links instead of an error.	97b09c6277a60f9b5b5a0d46e09da0bd3de15fa4

Summary

Fix the "watched list" resource.

The watched list resource is a special resource that exists only to link to the other watched item resources. It was completely broken, returning HTTP 405, due to a bad implementation of our singleton resource pattern. This change fixes that up to use the correct method name, and reimplements the body of it to skip the bulk of what the base `get_list` implementation would have done. Testing Done: Fetched the watched list resource and got a response with links instead of an error.

97b09c6277a60f9b5b5a0d46e09da0bd3de15fa4

Issues

Description	From	Last Updated
Given that this is a pretty big change to the resource (RootResource does a number of things), I think this …	chipx86	Dec. 13, 2024, 9:52 p.m.
'djblets.util.typing.JSONDict' imported but unused Column: 5 Error code: F401	reviewbot	Dec. 15, 2024, 9:01 p.m.
I don't think we want this typing here. If anything, it should be in the base class. The subclass inherits …	chipx86	Dec. 16, 2024, 4:41 p.m.
Can you add a module docstring while here?	chipx86	Dec. 16, 2024, 4:41 p.m.
Let's make these keyword-only.	chipx86	Dec. 16, 2024, 9:47 p.m.
'typing.TYPE_CHECKING' imported but unused Column: 1 Error code: F401	reviewbot	Dec. 16, 2024, 9:47 p.m.

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

The issue has been resolved. Show all issues

Given that this is a pretty big change to the resource (RootResource does a number of things), I think this change needs to come with more comprehensive tests that check the full output of the resource. I'm not 100% sure that this should be a Root Resource, really, since that does have its own implications, and I'd be weary of changes there having unexpected side effects here.

What error was returned? I feel like we should be starting with that.

Description:

~		We have a special resource that only exists to link to the other watched
~		item resources. Unfortunately, it was completely broken, and making a
~		GET request to it would just return an error.
	~	The watched list resource is a special resource that exists only to link
	~	to the other watched item resources. It was completely broken, returning
	~	HTTP 405, due to a bad implementation of our singleton resource pattern.

~		Other resources that we have that behave similarly are written as
~		inheriting from the Djblets RootResource. I've changed it so we do that
~		here, and now the resource works.
	~	This change fixes that up to use the correct method name, and
	~	reimplements the body of it to skip the bulk of what the base `get_list`
	~	implementation would have done.

Commits:

	Summary	ID
	Fix the "watched list" resource. We have a special resource that only exists to link to the other watched item resources. Unfortunately, it was completely broken, and making a GET request to it would just return an error. Other resources that we have that behave similarly are written as inheriting from the Djblets RootResource. I've changed it so we do that here, and now the resource works. Testing Done: Fetched the watched list resource and got a response with links instead of an error.	cdd447c3258923d05a9bf7249e737bd6e4459e7a
	Fix the "watched list" resource. The watched list resource is a special resource that exists only to link to the other watched item resources. It was completely broken, returning HTTP 405, due to a bad implementation of our singleton resource pattern. This change fixes that up to use the correct method name, and reimplements the body of it to skip the bulk of what the base `get_list` implementation would have done. Testing Done: Fetched the watched list resource and got a response with links instead of an error.	88abb87368503ad3c18f97f58b5e35113a55a0e5

Diff:

Revision 2 (+48 -8)

Show changes

reviewboard/webapi/resources/watched.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:

Add unit tests.

Commits:

	Summary	ID
	Fix the "watched list" resource. The watched list resource is a special resource that exists only to link to the other watched item resources. It was completely broken, returning HTTP 405, due to a bad implementation of our singleton resource pattern. This change fixes that up to use the correct method name, and reimplements the body of it to skip the bulk of what the base `get_list` implementation would have done. Testing Done: Fetched the watched list resource and got a response with links instead of an error.	88abb87368503ad3c18f97f58b5e35113a55a0e5
	Fix the "watched list" resource. The watched list resource is a special resource that exists only to link to the other watched item resources. It was completely broken, returning HTTP 405, due to a bad implementation of our singleton resource pattern. This change fixes that up to use the correct method name, and reimplements the body of it to skip the bulk of what the base `get_list` implementation would have done. Testing Done: Fetched the watched list resource and got a response with links instead of an error.	c3b82c23f4a7001935fee10f2953e288c98eafbb

Diff:

Revision 3 (+254 -10)

Show changes

	reviewboard/webapi/resources/watched.py
	reviewboard/webapi/tests/mimetypes.py
	reviewboard/webapi/tests/test_watched.py
	reviewboard/webapi/tests/urls.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

reviewboard/webapi/tests/test_watched.py (Diff revision 3)
The issue has been dropped. Show all issues
```
'djblets.util.typing.JSONDict' imported but unused

Column: 5
Error code: F401
```

Commits:

	Summary	ID
	Fix the "watched list" resource. The watched list resource is a special resource that exists only to link to the other watched item resources. It was completely broken, returning HTTP 405, due to a bad implementation of our singleton resource pattern. This change fixes that up to use the correct method name, and reimplements the body of it to skip the bulk of what the base `get_list` implementation would have done. Testing Done: Fetched the watched list resource and got a response with links instead of an error.	c3b82c23f4a7001935fee10f2953e288c98eafbb
	Fix the "watched list" resource. The watched list resource is a special resource that exists only to link to the other watched item resources. It was completely broken, returning HTTP 405, due to a bad implementation of our singleton resource pattern. This change fixes that up to use the correct method name, and reimplements the body of it to skip the bulk of what the base `get_list` implementation would have done. Testing Done: Fetched the watched list resource and got a response with links instead of an error.	e4eee6e7bca081a1aa363507e15ca6b418bad1e7

Diff:

Revision 4 (+250 -10)

Show changes

	reviewboard/webapi/resources/watched.py
	reviewboard/webapi/tests/mimetypes.py
	reviewboard/webapi/tests/test_watched.py
	reviewboard/webapi/tests/urls.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/webapi/tests/test_watched.py (Diff revision 4)

The issue has been resolved. Show all issues

I don't think we want this typing here. If anything, it should be in the base class. The subclass inherits the types. Having these here can result in conflicts fixing up the base class.

reviewboard/webapi/tests/urls.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Can you add a module docstring while here?
```
reviewboard/webapi/tests/urls.py (Diff revision 4)
The issue has been dropped. Show all issues
```
Let's make these keyword-only.
```
1. david Dec. 16, 2024, 4:15 p.m.
  I think I'd prefer to match everything else in this file.

Commits:

	Summary	ID
	Fix the "watched list" resource. The watched list resource is a special resource that exists only to link to the other watched item resources. It was completely broken, returning HTTP 405, due to a bad implementation of our singleton resource pattern. This change fixes that up to use the correct method name, and reimplements the body of it to skip the bulk of what the base `get_list` implementation would have done. Testing Done: Fetched the watched list resource and got a response with links instead of an error.	e4eee6e7bca081a1aa363507e15ca6b418bad1e7
	Fix the "watched list" resource. The watched list resource is a special resource that exists only to link to the other watched item resources. It was completely broken, returning HTTP 405, due to a bad implementation of our singleton resource pattern. This change fixes that up to use the correct method name, and reimplements the body of it to skip the bulk of what the base `get_list` implementation would have done. Testing Done: Fetched the watched list resource and got a response with links instead of an error.	95c44910f450c4a29f50f694429daad1815ad957

Diff:

Revision 5 (+248 -10)

Show changes

	reviewboard/webapi/resources/watched.py
	reviewboard/webapi/tests/mimetypes.py
	reviewboard/webapi/tests/test_watched.py
	reviewboard/webapi/tests/urls.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

reviewboard/webapi/tests/test_watched.py (Diff revision 5)
The issue has been dropped. Show all issues
```
'typing.TYPE_CHECKING' imported but unused

Column: 1
Error code: F401
```

Commits:

	Summary	ID
	Fix the "watched list" resource. The watched list resource is a special resource that exists only to link to the other watched item resources. It was completely broken, returning HTTP 405, due to a bad implementation of our singleton resource pattern. This change fixes that up to use the correct method name, and reimplements the body of it to skip the bulk of what the base `get_list` implementation would have done. Testing Done: Fetched the watched list resource and got a response with links instead of an error.	95c44910f450c4a29f50f694429daad1815ad957
	Fix the "watched list" resource. The watched list resource is a special resource that exists only to link to the other watched item resources. It was completely broken, returning HTTP 405, due to a bad implementation of our singleton resource pattern. This change fixes that up to use the correct method name, and reimplements the body of it to skip the bulk of what the base `get_list` implementation would have done. Testing Done: Fetched the watched list resource and got a response with links instead of an error.	97b09c6277a60f9b5b5a0d46e09da0bd3de15fa4

Diff:

Revision 6 (+248 -10)

Show changes

	reviewboard/webapi/resources/watched.py
	reviewboard/webapi/tests/mimetypes.py
	reviewboard/webapi/tests/test_watched.py
	reviewboard/webapi/tests/urls.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-7.x (55da94f)