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

Recipe ratings #342

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from
Draft

Recipe ratings #342

wants to merge 10 commits into from

Conversation

sam-19
Copy link
Contributor

@sam-19 sam-19 commented Oct 11, 2020

Here's a pull request for a recipe rating system (issue #323). This will be a collaborative effort, as work is needed both in the front-end and the back-end.

@sam-19
Copy link
Contributor Author

sam-19 commented Oct 11, 2020

First thing to decide is if we want to use the Rating type for the recipe rating or just a simple field. I would opt for the Rating type, because it is relatively simple but still has about everything necessary. bestRating would be 10 and worstRating 1, 0 is reserved for no rating. We can have a set of default ratingExplanations for when this field is left empty, but it's also good to give an option for a free text expression (such as "Great with macaroni and cheese!").

For the front-end, I would like to know if everyone is okay with using a third-party icon set? I assume we would use stars for the ratings and I have almost a copy-paste solution for a pretty nifty UI using the free FontAwesome icons. Using unicode characters is one option, of course, but controlling the look of the UI would be near impossible because it would depend on on the character set installed on the user's machine.

@christianlupus
Copy link
Collaborator

Regarding the type decision: I favor the Rating type as well. It allows us as well to link the ratings to NC users if necessary (e.g using author.identifier). As I understand things, the JSON is sent "as-it-is" to the Vue components and parsed/rendered there, right? So there seem not to be much changes in the backend required as I see it. Please tell me, if and what I might assist here.

Regarding the icon set, I am ok with you solution as long as the usage is free for open source.

@christianlupus
Copy link
Collaborator

@sam-19, just to be clear: What exactly is needed from the backend perspective? It would maybe be a good first step if we define the interface (together). If I find a few free minutes, I might be able to implement the backend part soon. This might help you with a functional (hopefully) backend to test the frontend.

@sam-19
Copy link
Contributor Author

sam-19 commented Oct 16, 2020

Sorry, I'm really busy at the moment, I'll try to get working on this as soon as possible. The backend would of course have to save the ratings with the recipes, preferably in the recipe JSON file. When a recipe is loaded (for viewing or editing) the numerical value of the rating (ratingValue) and possible short description (ratingExplanation) should be returned. So the API could inject an attribute like this into the recipe data
contentRating: { ratingValue <int 0-10>, ratingExplanation <string> }
and a similar attribute would be sent when a recipe is saved. Metadata like minimum and maximum rating are not needed in the frontend.

The schema.org Rating includes also the field author, and this may be a bit tricky to handle on the backend. So to remain faithful to the original proposal, each contentRating field should in fact be an array of Rating objects, one for each person who have rated the recipe. First problem arises, if the recipe is shared but some people only have read access. Should they not be able to rate? Should their ratings be saved in a database entry? And what if someone wants their rating to remain private, even if the recipe is shared (this would we impossible if the recipes are saved as plain text on the file system)? I think some compromise will be necessary. Maybe a good place to start would be to only pass each user's own ratings through the API and maybe implement something with a combined or average rating later. Also, if we want ratings to be displayed on the index and/or search page recipe listing, those objects need to at least include the ratingValue.

@christianlupus christianlupus linked an issue Oct 21, 2020 that may be closed by this pull request
@christianlupus
Copy link
Collaborator

OK, this might get a bit lengthy, sorry for that. I try to organize this a bit and separate issues here.

Where should the ratings be stored?

I think the main credo for this app is to save everything into the JSON files as the DB is considered to break more easily. Thus, I suggest that the JSON file is going to be changed upon each recipe rating.

I think we need to define a new API endpoint for CRUD operation of the user's (single) rating of a (single) recipe. I suggest to locate these under /api/recipes/{id}/rating. This would update the JSON file in the backend both for the contentRating as well as aggregateRating.

@sam-19 if you do not mind, I will just define the routes accordingly in the routes.php as a starting point and as an interface decision in the next days.

Sharing of recipes and ratings

Currently the only way to share recipes is by sharing the whole folder of recipes.
I have the plan to change here some structures in the backend in the future (#340, #301) that should allow a more detailed structure. With these changes a sharing on per-recipe-basis might be realizable.

Until these changes are carried out, I suggest to stick with a simple sharing mechanism as well for the ratings. Each user can rate a recipe at most once (update is of course allowed but only one rating entity). The user can be identified by Rating > author > identifier. I suggest to save this identifier to something like nextcloud-cookbook-{uid} where {uid} is the name of the nextcloud user. Then in the backend the Person > name can be set according to the NC user's alias.

Any change in the ratings causes the backend to recalculate the aggregated ratings in the JSON. That way, it needs to be calculated only once.

Viewing of the ratings of the recipe

When it comes to presenting the recipe to the user, the current state of using the JSON as a transport medium can be kept. The ratings are to be embedded into the structure at appropriate keys. So showing of the data should be straight forward by just extending the vue components.

Adding other views and searching

Another issue is sorting by recipes (at least rating of x) and filtering etc. Here I suggest to keep the effort at minimum for now. The list of recipes is delivered with a list of recipes as far as I know (I have not checked at the moment). Thus, all information should be available at the corresponding places. If something is missing we can add more data or more API endpoints on demand.

@christianlupus
Copy link
Collaborator

I wanted to suggest to migrate the API endpoints to versioned ones. So instead of /api/recipes/.... I suggest to use /api/v1/recipes. I do not intend to change all routes but only those newly inserted in the PR. What do you think of this, @sam-19?

@sam-19
Copy link
Contributor Author

sam-19 commented Oct 25, 2020

This all sounds good, @christianlupus! A versioned API pretty much is mandatory for RESTful implementations, if anyone ever wants to extend this application that way, and not a bad idea even if that never happens. I was wondering if we could get away with using a resource instead of separate routes for the rating, i.e. "/api/v1/ratings"? That would require that you can catch those requests somewhere along the chain in the backend and substitute the rating ID with the recipe ID.

PS: I'm not ignoring this PR or your review requests, just need a couple more days to handle some urgent matters.

@christianlupus
Copy link
Collaborator

This all sounds good, @christianlupus! A versioned API pretty much is mandatory for RESTful implementations, if anyone ever wants to extend this application that way, and not a bad idea even if that never happens.

OK, I will update that accordingly soon. The next days I am a bit short in time but I will move towards versioned API endpoints.

I was wondering if we could get away with using a resource instead of separate routes for the rating, i.e. "/api/v1/ratings"? That would require that you can catch those requests somewhere along the chain in the backend and substitute the rating ID with the recipe ID.

My main intention was to use a special API and reject any changes made by /recipes/{id}/edit on the set of ratings. So any user can at most affect his own ratings. To realize that I just added the required endpoints. The reading is done implicitly by reading the recipe using /recip[es/{id}.
As the rating is uniquely identifiable by userId and recipeId, it is not required to map to a ratingId. I am not yet sure about the best DB structure here. Depends if the DBAL allows for multi-column keys/indices (I have to take a look there). Eventually, we can drop the ratingId column completely.

Or do you mean to use a service and reinterpret the id in the service endpoint as recipeId instead of the ratingId (merely against the basic idea of a service to be a shortcut for a certain set of routes with an id as identifier)?

PS: I'm not ignoring this PR or your review requests, just need a couple more days to handle some urgent matters.

I see. No issue here for me. I just did not want to wilder in your main topic.
BTW: Are you in the matrix channel? That might be better suited for short informations and questions like this.

@christianlupus
Copy link
Collaborator

The current state I did not test yet but it should work. I wanted to avoid to do the migration involved as this will potentially involve more work to keep the dev environment ready for other tests.
@sam-19 The question is, which routes are needed to finish this up from the frontend perspective. Of course that depends a bit how far you need to go and how much information is going to be stored in the Vuex storage. Please give me a short comment about that. Thank you.

sam-19 and others added 10 commits November 15, 2020 17:59
Signed-off-by: sam-19 <26563023+sam-19@users.noreply.github.com>
Signed-off-by: Christian Wolf <github@christianwolf.email>
Signed-off-by: Christian Wolf <github@christianwolf.email>
Signed-off-by: Christian Wolf <github@christianwolf.email>
Signed-off-by: Christian Wolf <github@christianwolf.email>
Signed-off-by: Christian Wolf <github@christianwolf.email>
… file

Signed-off-by: Christian Wolf <github@christianwolf.email>
Signed-off-by: Christian Wolf <github@christianwolf.email>
Signed-off-by: Christian Wolf <github@christianwolf.email>
Signed-off-by: Christian Wolf <github@christianwolf.email>
@patn03
Copy link

patn03 commented Jan 5, 2021

@sam-19 is this something you can help get tested and or finished?

@christianlupus If sam is not coming back what would be left to get this some what functional? User based and Versioned rest URIs? Thanks, An interested party member

@christianlupus
Copy link
Collaborator

@patn03 I think the next steps would be to define the UI interface (where should be visible what). Starting from there the UX must be implemented and any missing parts in the API can be identified. After implementing these, we are good to go to have a test.

@sam-19
Copy link
Contributor Author

sam-19 commented Jan 10, 2021

Hey, and sorry I haven't been able to contribute in a while. There are a couple of other projects that take up practically all of my available time at the moment. I started this PR because I already had an almost identical component working in another Vue project. To save some time, I will give some hints on how this can be implemented in a simple and robust way (minimal javascript).

It uses two icons, one for a solid star shape and one for an outline shape. Additionally, if we want to use half stars (and I suggest we should) it also uses a solid half-star and and outline of a half-star. Before I started this PR I checked that at least FontAwesome should have the required icons available in their free package.

Below is the Vue component template part. My example uses the font-awesome-icon component but it can just as easily be made using the < i > tags.

<div class="rating">
    <div class="rating-stars">
        <label v-for="(v,i) in 11" :key="'hl'+i" @dblclick="clearRating()">
            <input v-model="rating"
                :class="'rating'+i"
                type="radio" name="rating"
                :value="i" @click="i%2 ? null : halfStar(i)"
            />
            <font-awesome-icon v-for="n in (i-i%2)"
                 :key="'fs'+n"
                 :icon="[n%2 ? 'fas' : 'fal','star']"
                 :class="n%2 ? 'star-fill' : 'star-outline'"
            />
            <font-awesome-icon v-if="i%2" :icon="['fas','star-half']" class="star-fill" />
            <font-awesome-icon v-if="i%2" :icon="['fal','star-half']" class="star-outline" />
        </label>
        <div>
            <font-awesome-icon v-for="n in 5" :key="'bg'+n" icon="star" />
        </div>
    </div>
    <input type="text" class="rating-desc" :placeholder="$t('general.rating_descriptions')[rating]" v-model="ratingDesc" />
</div>

Idea being that we first add a radio input with 10 options that we bind the rating property to, then on top of them display the correct amount of stars (depending on which option is selected, this is done using CSS below). The last for-loop creates 5 light gray stars in the background to show in place of the "missing" stars. Clicking the last full star will remove half of the last star (e.g. if the rating is 3 stars, clicking the third star will make it 2,5 stars) and double clicking any star will reset the rating to zero. After the star icons there is a text field for an optional description.

In the methods part of the component:

clearRating: function () {
    this.rating = 0
},
halfStar: function (r) {
    // Turn full star into half star
    if (this.rating === r) {
        this.rating -= 1
    }
},

The CSS part:

.rating {
    position: relative;
}
.rating-stars {
    display: inline-block;
    position: relative;
}
.rating input[type=text] {
    /* This is the optional description field */
    position: absolute;
    left: 5.5em;
    width: whatever
}
/* Ignore clicks on icons and radio inputs (leaving only the labels to interact with) */
.rating-stars svg,
.rating-stars label input[type=radio] {
    pointer-events: none
}
/* Position the first label and first child div (gray background stars) */
.rating-stars label {
    position: absolute;
    top: 0;
    left: 0;
    height: 1em;
    cursor: pointer;
}
.rating-stars div {
    position: absolute;
    top: 0;
    left: 0;
    width: 5.1em;
    height: 1em
}
/* Layer the consecutive labels on top of each other */
/* Even numbers are full stars */
.rating-stars label:nth-child(2) {
    z-index: 9;
    width: 1.1em
}
.rating-stars label:nth-child(4) {
    z-index: 7;
    width: 2.1em
}
.rating-stars label:nth-child(6) {
    z-index: 5;
    width: 3.1em
}
.rating-stars label:nth-child(8) {
    z-index: 3;
    width: 4.1em
}
.rating-stars label:nth-child(10) {
    z-index: 1;
    width: 5.1em
}
/* Odd numbers are half stars */
.rating-stars label:nth-child(3) {
    z-index: 10;
    width: 1.1em
}
.rating-stars label:nth-child(5) {
    z-index: 8;
    width: 2.1em
}
.rating-stars label:nth-child(7) {
    z-index: 6;
    width: 3.1em
}
.rating-stars label:nth-child(9) {
    z-index: 4;
    width: 4.1em
}
.rating-stars label:nth-child(11) {
    z-index: 2;
    width: 5.1em
}
/* Hide the radio inputs */
.rating-stars label input {
    position: absolute;
    top: 0;
    left: 0;
    opacity: 0;
}
/* Full stars must cover their underlying half-stars */
.rating-stars label input.fullstar {
    z-index: 2
}
/* Make the icons invisible by default */
.rating-stars label svg {
    position: absolute;
    color: transparent;
}
/* Background stars should be visible */
.rating-stars div svg {
    color: #000000; /* or some other color */
    opacity: 0.2;
}
/* Position the stars inside their containers */
/* Each container has stars up to the rating it represents */
    .rating-stars label svg:nth-child(2),
    .rating-stars label svg:nth-child(3),
    .rating-stars div svg:nth-child(1) {
        left: 0
    }
    .rating-stars label svg:nth-child(4),
    .rating-stars label svg:nth-child(5),
    .rating-stars div svg:nth-child(2) {
        left: 1em
    }
    .rating-stars label svg:nth-child(6),
    .rating-stars label svg:nth-child(7),
    .rating-stars div svg:nth-child(3) {
        left: 2em
    }
    .rating-stars label svg:nth-child(8),
    .rating-stars label svg:nth-child(9),
    .rating-stars div svg:nth-child(4) {
        left: 3em
    }
    .rating-stars label svg:nth-child(10),
    .rating-stars label svg:nth-child(11),
    .rating-stars div svg:nth-child(5) {
        left: 4em
    }
/* Finally, make only the stars inside the selected/hovered label visible */
.rating-stars:hover label:hover input ~ svg.star-fill,
.rating-stars:not(:hover) label input:checked ~ svg.star-fill {
    color: some-star-color
}
/* Make the star outline a bit darker */
.rating-stars:hover label:hover input ~ svg.star-outline,
.rating-stars:not(:hover) label input:checked ~ svg.star-outline {
    color: #000000;
    opacity: 0.2; /* or some other opacity */
}

This was made for another project and may not translate flawlessly, but it should already get you pretty far.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rate recipes
3 participants