Skip to content

Commit

Permalink
Addons: resolve URLs for file tree diff API response (#11749)
Browse files Browse the repository at this point in the history
Resolve all the filename into URLs, so the frontend can use these links
without manipulating them. We don't want re-implement the resolver in
the frontend.

Now, we return the filename, the URL for the current version, and the
URL for the source version of the filename changed.

Closes readthedocs/addons#409
  • Loading branch information
humitos authored Nov 12, 2024
1 parent b207d2e commit 9a14ae9
Show file tree
Hide file tree
Showing 3 changed files with 116 additions and 23 deletions.
20 changes: 17 additions & 3 deletions readthedocs/filetreediff/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,24 @@ def get_diff(version_a: Version, version_b: Version) -> FileTreeDiff | None:
if file_a.main_content_hash != file_b.main_content_hash:
files_modified.append(file_path)

def sortpath(filename):
"""
Function to use as `key=` argument for `sorted`.
It sorts the file names by the less deep directories first.
However, it doesn't group the results by directory.
Ideally, this should sort file names by hierarchy (less deep directory
first), groupping them by directory and alphabetically. We should update
this function to achieve that goal if we find a simple way to do it.
"""
parts = filename.split("/")
return len(parts), parts

return FileTreeDiff(
added=files_added,
deleted=files_deleted,
modified=files_modified,
added=sorted(files_added, key=sortpath),
deleted=sorted(files_deleted, key=sortpath),
modified=sorted(files_modified, key=sortpath),
outdated=outdated,
)

Expand Down
55 changes: 40 additions & 15 deletions readthedocs/proxito/tests/test_hosting.py
Original file line number Diff line number Diff line change
Expand Up @@ -925,27 +925,52 @@ def test_file_tree_diff(self, get_manifest):
],
),
]
r = self.client.get(
reverse("proxito_readthedocs_docs_addons"),
{
"url": "https://project--123.dev.readthedocs.build/en/123/",
"client-version": "0.6.0",
"api-version": "1.0.0",
},
secure=True,
headers={
"host": "project--123.dev.readthedocs.build",
},
)
with self.assertNumQueries(30):
r = self.client.get(
reverse("proxito_readthedocs_docs_addons"),
{
"url": "https://project--123.dev.readthedocs.build/en/123/",
"client-version": "0.6.0",
"api-version": "1.0.0",
},
secure=True,
headers={
"host": "project--123.dev.readthedocs.build",
},
)
assert r.status_code == 200
filetreediff_response = r.json()["addons"]["filetreediff"]
assert filetreediff_response == {
"enabled": True,
"outdated": False,
"diff": {
"added": [{"file": "new-file.html"}],
"deleted": [{"file": "deleted.html"}],
"modified": [{"file": "tutorial/index.html"}],
"added": [
{
"filename": "new-file.html",
"urls": {
"version_a": "https://project--123.dev.readthedocs.build/en/123/new-file.html",
"version_b": "https://project.dev.readthedocs.io/en/latest/new-file.html",
},
},
],
"deleted": [
{
"filename": "deleted.html",
"urls": {
"version_a": "https://project--123.dev.readthedocs.build/en/123/deleted.html",
"version_b": "https://project.dev.readthedocs.io/en/latest/deleted.html",
},
},
],
"modified": [
{
"filename": "tutorial/index.html",
"urls": {
"version_a": "https://project--123.dev.readthedocs.build/en/123/tutorial/index.html",
"version_b": "https://project.dev.readthedocs.io/en/latest/tutorial/index.html",
},
},
],
},
}

Expand Down
64 changes: 59 additions & 5 deletions readthedocs/proxito/views/hosting.py
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,10 @@ def _v1(self, project, version, build, filename, url, request):

if version:
response = self._get_filetreediff_response(
request=request, project=project, version=version
request=request,
project=project,
version=version,
resolver=resolver,
)
if response:
data["addons"]["filetreediff"].update(response)
Expand Down Expand Up @@ -614,7 +617,7 @@ def _v1(self, project, version, build, filename, url, request):

return data

def _get_filetreediff_response(self, *, request, project, version):
def _get_filetreediff_response(self, *, request, project, version, resolver):
"""
Get the file tree diff response for the given version.
Expand All @@ -641,9 +644,60 @@ def _get_filetreediff_response(self, *, request, project, version):
"enabled": True,
"outdated": diff.outdated,
"diff": {
"added": [{"file": file} for file in diff.added],
"deleted": [{"file": file} for file in diff.deleted],
"modified": [{"file": file} for file in diff.modified],
"added": [
{
"filename": filename,
"urls": {
"version_a": resolver.resolve_version(
project=project,
filename=filename,
version=version,
),
"version_b": resolver.resolve_version(
project=project,
filename=filename,
version=latest_version,
),
},
}
for filename in diff.added
],
"deleted": [
{
"filename": filename,
"urls": {
"version_a": resolver.resolve_version(
project=project,
filename=filename,
version=version,
),
"version_b": resolver.resolve_version(
project=project,
filename=filename,
version=latest_version,
),
},
}
for filename in diff.deleted
],
"modified": [
{
"filename": filename,
"urls": {
"version_a": resolver.resolve_version(
project=project,
filename=filename,
version=version,
),
"version_b": resolver.resolve_version(
project=project,
filename=filename,
version=latest_version,
),
},
}
for filename in diff.modified
],
},
}

Expand Down

0 comments on commit 9a14ae9

Please sign in to comment.