tournesol icon indicating copy to clipboard operation
tournesol copied to clipboard

[back, front] feat: Rate later list display information about the number of comparisons

Open lfaucon opened this issue 3 years ago • 1 comments

As a user I want to see how many times I have compared the videos when I look at my rate later list so that I can easily chose which videos to remove from my rate later list.

Display the information similarly as in the card used on the comparison page.

This probably will be best implemented by updating the backend endpoint to include the extra info

lfaucon avatar Feb 18 '22 20:02 lfaucon

See the plan described in the PR https://github.com/tournesol-app/tournesol/pull/1055

GresilleSiffle avatar Jul 25 '22 15:07 GresilleSiffle

Here is the JSON the API could return.

The new poll_metadata key contains all data related to the poll, like the total number of comparisons by all contributors. It can also be called collective_metadata but poll is more accurate as it matches the related back end's model.

The user_metadata key contains all data related to the logged user. Having different keys for collective and and personal data will allow the front end to: (1) know what kind of data are displayed ; (2) display both collective and personal data at the same time.

The goal of such representation is to avoid adding unrelated keys at the root of the JSON, making the variable types managed by the front end unnecessarily complex. Here the entity object has always the same structure, poll_metadata and user_metadata too.

{
  "entity": {
    "uid": "string",
    "type": "video",
    "metadata": {
      "additionalProp1": "string",
      "additionalProp2": "string",
      "additionalProp3": "string"
    }
  },
  "poll_metadata": {
    "name": "string",
    "tournesol_score": 0,
    "n_comparisons": 0,
    "n_contributors": 0
  },
  "user_metadata": {
    "n_comparisons": 0,
    "n_contributors": 0
  },
  "rate_later_metadata": {
    "created_at": "2022-11-08T13:47:39.480Z"
  }
}

API also returning criteria scores could use the following JSON:

{
  "entity": {
    "uid": "string",
    "type": "video",
    "metadata": {
      "additionalProp1": "string",
      "additionalProp2": "string",
      "additionalProp3": "string"
    }
  },
  "poll_metadata": {
    "name": "string",
    "tournesol_score": 0,
    "n_comparisons": 0,
    "n_contributors": 0,
    "criteria_scores": []
  },
  "user_metadata": {
    "n_comparisons": 0,
    "n_contributors": 0,
    "criteria_scores": []
  }
}

By following this rule (no wild key at the root of the JSON object), we can also simplify the /entities/ API which currently add the polls key directly inside the entity object.

current payload

(the tournesol_score is embedded in the entity object and the polls don't contain ratings metadata)

{
  "uid": "string",
  "type": "video",
  "metadata": {
    "additionalProp1": "string",
    "additionalProp2": "string",
    "additionalProp3": "string"
  },
  "tournesol_score": 0,
  "polls": [
    {
      "name": "string",
      "criteria_scores": [
        {
          "criteria": "string",
          "score": 0
        }
      ]
    }
  ]
}

suggested payload

(the entity is delimited and compatible with all front end's components using entities as props, the poll metadata are delimited for the same reasons)

{
  "entity": {
    "uid": "string",
    "type": "video",
    "metadata": {
      "additionalProp1": "string",
      "additionalProp2": "string",
      "additionalProp3": "string"
    }
  },
  "polls": [
    {
      "name": "string",
      "tournesol_score": 0,
      "n_comparisons": 0,
      "n_contributors": 0,
      "criteria_scores": [
        {
          "criteria": "string",
          "score": 0
        }
      ]
    }
  ]
}

GresilleSiffle avatar Nov 08 '22 13:11 GresilleSiffle

I just remove the labels Good first issue and Hacktoberfest as the proposed rework requires a global view of the current API's architecture.

GresilleSiffle avatar Nov 08 '22 14:11 GresilleSiffle

User metadata is also poll specific. The distinction is rather at the level or global/community/aggregated vs. user/personal ? Not poll vs. user.

lfaucon avatar Nov 08 '22 16:11 lfaucon

Apart from this, I really like the proposal to simplify the keys at the root of the entity JSON representations and allowing API routes to include the root entries or not depending on what is requested.

I would even go further and impose fixed types for each of the root entries. For example by having one entry global_metadata and one global_criteria_scores to avoid changing the type of the global_metadata entry

lfaucon avatar Nov 08 '22 16:11 lfaucon

You have n_contributors in entry user_metadata. Is it a typo or something that I don't understand correctly?

lfaucon avatar Nov 08 '22 16:11 lfaucon

Thanks @GresilleSiffle for the details, I think it's a step in the good direction.

User metadata is also poll specific. The distinction is rather at the level or global/community/aggregated vs. user/personal ? Not poll vs. user.

I agree. "poll_metadata" and "user_metadata" could bring more confusion. For example, a "tournesol_score" (contrary to "name") is not a property of a Poll, it's part of an EntityPollRating. And keeping this distinction clear will also make the implementation of the serializers easier.

Also, about naming, I would not use "_metadata" as a suffix for keys. It does not seem like to good description to me. Either "user_metadata" represents the user and could be named "user", or its name should be more explicit.

So based on the first suggestion, I would suggest something like:

{
  "entity": {
    "uid": "string",
    "type": "video",
    "metadata": {
      "additionalProp1": "string",
      "additionalProp2": "string",
      "additionalProp3": "string"
    }
  },
  "poll": {
  // represents an instance of `Poll`
    "name": "string",
  },
  "global": { // or "global_rating", or  "poll_rating" ?
  // represents an instance of `EntityPollRating`, could be extended with `"criteria_scores"` in some contexts
    "tournesol_score": 0,
    "n_comparisons": 0,
    "n_contributors": 0
  },
  "individual": { // or "contributor_rating" ?
   // represents an instance of `ContributorRating`, could be extended with `"criteria_scores"` in some contexts
    "n_comparisons": 0,
  },
  "rate_later_metadata": {
    "created_at": "2022-11-08T13:47:39.480Z"
  }
}

amatissart avatar Nov 08 '22 17:11 amatissart

Thank you!

You're right, using the suffix _metadata is not that good.

I like collective_rating instead of global / global_rating / poll_rating. The _rating might disappear the day we will save more information than just ratings, but keeping it right now make it more explicit tha simply collective or global.

If individual represents the contributor rating, maybe we can use the name contributor_rating directly. We can try to re-use the existing ContributorRatingSerializer or a similar read-only oriented serializer.

edit: finally i prefer invididual_rating rather than contributor_rating as the JSON representation differs enough from the ContributorRatingSerializer (not entity field, no poll field, no last_compared_at field, etc.)

Based on the last suggestion, what do you think of this?

{
  "entity": {
    "uid": "string",
    "type": "video",
    "metadata": {
      "additionalProp1": "string",
      "additionalProp2": "string",
      "additionalProp3": "string"
    }
  },
  "poll": {
    "name": "string",
  },
  "invididual_rating": {
    "is_public": true,
    "n_comparisons": 0
  },
  "collective_rating": {
    "tournesol_score": 0,
    "n_comparisons": 0,
    "n_contributors": 0
  },
  "rate_later_metadata": {
    "created_at": "2022-11-08T13:47:39.480Z"
  }
}

GresilleSiffle avatar Nov 09 '22 07:11 GresilleSiffle

It looks like we will be able to re-use this new structure in the contributor ratings API. The only difference is the field rate_later_metadata which transforms into a contributor_rating_metadata to provide new fields like last_compared_at.

GresilleSiffle avatar Nov 09 '22 08:11 GresilleSiffle

Merging this PR will help to address the current issue: https://github.com/tournesol-app/tournesol/pull/1149/files

The PR will ensure the models EntityPollRatings contain data the can be returned in the collective_rating field of the rate-later JSON. It's not directly related to the issue, but it will avoid doing two refactor in a row of the same serializer.

GresilleSiffle avatar Nov 09 '22 08:11 GresilleSiffle

Left to-do

  • [x] Adapt the ratings API:
    #1745
  • [x] Adapt the recommendations API: #1774
  • [x] update the extension to use the new entity, and collective_ratings fields
    • addressed by #1806
  • [x] Adapt front rate-later page video cards to display the number of comparisons
  • [ ] remove the legacy fields from the API at the end #1854

lfaucon avatar Aug 29 '23 17:08 lfaucon