WIP: Feature/154 move ratings to three point grading scale

[sekl]

Closes #154

• Changed constant and helpers for tests to reflect new rating system
• Updated tests for ratings and reputation rewards/penalties
• Making sure only ratings 1,2,3 can be given
• Since we can now use a role's rating to find out if they have been rated or not by comparing with 0, I've removed their rated attribute and am handling all this logic in finalizeTask instead.
• In finalizeTask, for each role, I keep track of the submitted rating, as it can be 0, and set the role.rating accordingly (to 3 if necessary). Then I award reputation based on the received rating. Only after that is done do I assign reputation penalties if necessary, and lastly I moved the worker's payout reduction (in case they didn't submit a rating) to the end because all the reputation updates depend on it.

Still WIP:

• Double check all the new rating logic and add missing tests, fix rest of tests
• Evaluator reputation reward currently doesn't make sense, as they always receive at least half the payout worth of reputation even if they didn't submit a rating.

[area]

Is there a reason to keep ratedOnTime, and not just check against .rating > 0 where it's needed? On-chain storage is expensive, so cutting it wherever possible is beneficial!

[sekl]

Yes and no. >0 doesn’t mean rated on time because the rating is set to 3 when the other party didn’t rate. As the whole process is spread out over multiple steps we need to keep track of it.

However we could use a rating of 4 (meaning rating 3 and late) instead of a separate variable and then handle it in ‘finalizeTask’. That would save on storage but make the code a lot less clear. I will check if it’s feasible later if you think its better.

[elenadimitrova]

Since we're no longer doing rating deductions do we still need to assignWorkRating? Can we work with finalizeTask logic only for reputation penalties?
Also since a rating can no longer be a 0 we can potentially remove the Role.rated property.

[sekl]

I'm not anywhere near my computer today but I'll check once I get the chance. I think if assignWorkRating is the step that sets the ratings to 3 if there hasn't been a rating, then yes we can probably remove that completely and that makes any other way of tracking ratedOnTime unnecessary. Thanks for the hint, both of you, will change it tomorrow.

[sekl]

@area @elenadimitrova I've removed assignWorkRating and am handling that logic in finalizeTask now as you suggested. I'm also changing the taskWorkRatingsClosed modifier so it checks if either the task has been rated as normal or the date has passed, so that hopefully the flow for a task still makes sense.

I'm still fixing up the tests, but maybe you can already see if this is still as intended.

[elenadimitrova]

In cases where rating is 0 , reputation will be 0 here right? Shouldn't the correct missing ratings be set before we calculate reputation?

[sekl]

Yes, thanks, I just need to move line 280 below the reputation penalty I think. Will change it soon.

[elenadimitrova]

The logic that follows here isn't very clear to me and I think there's an issue in that you are penalising the skill reputation of the worker (if they haven't submitted) but then you are also awarding them the full reputation (line 301).
The rules are: everyone earns domain reputation, worker earns skill reputation. Evaluator and worker get penalised on domain and domain and skill reputation respectively if they don't rate on time.

[sekl]

The logic in finalizeTask is getting quite complex so I’m trying to be as explicit as possible. I am still having issues with:

• tests for the reputation updates don’t pass. I think I’m doing something wrong with giving the negative reputation update, as one test shows reputation update of 0 instead of a negative number.
• Also it’s quite difficult to test which reputation log update is the correct one because all we know about them is the order and user it belongs to?
• And I’m not sure what to do in case the evaluator did not submit a rating on time, because currently this means they will get the normal reputation as if they had rated, and then they will lose half of that reputation again for not rating. Doesn’t make sense.

[sekl]

Still to do:

• Split the loop in finalizeTask into 3 internal functions to handle manager, evaluator, worker reputation updates. That will make it easier to penalize roles that have not rated.
• Evaluator should get an implied rating of 1 if they didn't rate the worker plus the separate rating penalty.

[area]

Now done by #228. Thanks for your input on this though, @sekl - I know that @kronosapiens looked through your PR for anything that he'd missed, and some of the issues you brought up here now live on in #232

[kronosapiens]

Yes, your work on this was a great reference @sekl 🙇

[sekl]

Awesome, glad it got done 👍