Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add torrent name to torrent list an detail endpoints #265

Merged

Conversation

mario-nt
Copy link
Contributor

Resolves #264

@josecelano josecelano changed the title #264 wrong downloaded torrent file name Add torrent name to torrent list an detail endpoints Aug 30, 2023
@josecelano
Copy link
Member

josecelano commented Aug 30, 2023

Hi @mario-nt It looks good to me. We are using Conventional Commits, maybe we can merge all commits in a single commit with a message like this:

feat: [#264] add torrent name to list and detail endpoints

The list and detail torren endpoints now contain the torrent name:

For example:

{
    "data": {
        "torrent_id": 175,
        "uploader": "admin",
        "info_hash": "124f00ff15f00ae19d7b939950090cb42ab6e852",
        "title": "www",
        "description": "www",
        "category": {
            "category_id": 63,
            "name": "Paper",
            "num_torrents": 4
        },
        "upload_date": "2023-08-26 20:02:27",
        "file_size": 639140,
        "seeders": 0,
        "leechers": 0,
        "files": [
            {
                "path": [
                    "mandelbrot_set_07"
                ],
                "length": 639140,
                "md5sum": null
            }
        ],
        "trackers": [
            "udp://localhost:6969",
            "udp://localhost:6969"
        ],
        "magnet_link": "magnet:?xt=urn:btih:124f00ff15f00ae19d7b939950090cb42ab6e852&dn=www&tr=udp%3A%2F%2Flocalhost%3A6969&tr=udp%3A%2F%2Flocalhost%3A6969",
        "tags": [],
        "name": "mandelbrot_set_07"
    }
}


Notice the last field `name`. That field is in the table column `torrust_torrents::name`.

It would or will be used in the frontend to set the name for the downloaded torrent file.

The list and detail torrent endpoints now contain the torrent name:

For example:

{
    "data": {
        "torrent_id": 175,
        "uploader": "admin",
        "info_hash": "124f00ff15f00ae19d7b939950090cb42ab6e852",
        "title": "www",
        "description": "www",
        "category": {
            "category_id": 63,
            "name": "Paper",
            "num_torrents": 4
        },
        "upload_date": "2023-08-26 20:02:27",
        "file_size": 639140,
        "seeders": 0,
        "leechers": 0,
        "files": [
            {
                "path": [
                    "mandelbrot_set_07"
                ],
                "length": 639140,
                "md5sum": null
            }
        ],
        "trackers": [
            "udp://localhost:6969",
            "udp://localhost:6969"
        ],
        "magnet_link": "magnet:?xt=urn:btih:124f00ff15f00ae19d7b939950090cb42ab6e852&dn=www&tr=udp%3A%2F%2Flocalhost%3A6969&tr=udp%3A%2F%2Flocalhost%3A6969",
        "tags": [],
        "name": "mandelbrot_set_07"
    }
}

Notice the last field `name`. That field is in the table column `torrust_torrents::name`.

It's going to be used in the frontend to set the name for the downloaded torrent file.
@mario-nt mario-nt force-pushed the 264-wrong-downloaded-torrent-file-name branch from 2c4b757 to f2369b4 Compare September 2, 2023 16:22
@josecelano josecelano self-requested a review September 4, 2023 15:58
Copy link
Member

@josecelano josecelano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @mario-nt there is still something that needs to be added. You should update the tests:

#[derive(Deserialize, PartialEq, Debug)]
pub struct ListItem {
    pub torrent_id: i64,
    pub uploader: String,
    pub info_hash: String,
    pub title: String,
    pub description: Option<String>,
    pub category_id: i64,
    pub date_uploaded: String,
    pub file_size: i64,
    pub seeders: i64,
    pub leechers: i64,
}

#[derive(Deserialize, PartialEq, Debug)]
pub struct TorrentDetails {
    pub torrent_id: Id,
    pub uploader: String,
    pub info_hash: String,
    pub title: String,
    pub description: String,
    pub category: Category,
    pub upload_date: UtcDateTime,
    pub file_size: u64,
    pub seeders: u64,
    pub leechers: u64,
    pub files: Vec<File>,
    pub trackers: Vec<String>,
    pub magnet_link: String,
}

In fact, the TorrentDetails does not contain the tags either. Those structs should contain the data we have in the endpoints now.

In the future, we could extract a package for the API contract, so that we do not need to add the new fields twice in the production and testing code.

Added the field name to the TorrentDetails, ListItem structs used in the response
from the API calls (list and detail endpoints) in the tests.

Some minor refactoring was also applied so the code used to test that the torrents info
is retrieve correctly and match the expected result.
@josecelano
Copy link
Member

ACK 7c4b530

@codecov-commenter
Copy link

Codecov Report

Merging #265 (7c4b530) into develop (79ff3cc) will decrease coverage by 0.07%.
Report is 6 commits behind head on develop.
The diff coverage is 0.00%.

@@             Coverage Diff             @@
##           develop     #265      +/-   ##
===========================================
- Coverage    44.80%   44.73%   -0.07%     
===========================================
  Files           77       77              
  Lines         4111     4124      +13     
===========================================
+ Hits          1842     1845       +3     
- Misses        2269     2279      +10     
Files Changed Coverage Δ
src/databases/mysql.rs 0.00% <ø> (ø)
src/databases/sqlite.rs 23.61% <ø> (ø)
src/models/response.rs 0.00% <0.00%> (ø)
src/models/torrent.rs 19.04% <0.00%> (-0.96%) ⬇️

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@josecelano josecelano merged commit 9ba65cc into torrust:develop Sep 11, 2023
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

The downloaded torrent filename is the torrent title instead of the original torrent file name
3 participants