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

accommodation rating system #165

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

cstns
Copy link
Contributor

@cstns cstns commented Mar 5, 2022

Requirements for making a pull request

Thank you for contributing to our project!

Please fill out the template below to help the project maintainers review it as fast as possible and include your contribution to the project.

What does it fix?

Closes #151

Please mention the main changes this PR brings.

How has it been tested?

Please describe the tests that you ran to verify your changes.

@andreiio andreiio marked this pull request as draft March 5, 2022 13:01
- added newer version & css of jquery plugin
- added am accommodation calculated rating property
- displayed rating on the accommodation & for reviews
@cstns cstns marked this pull request as ready for review March 6, 2022 00:33
@cstns cstns changed the title WIP accommodation rating system accommodation rating system Mar 6, 2022
@cstns
Copy link
Contributor Author

cstns commented Mar 6, 2022

should be ok for review

@beniamin
Copy link
Contributor

beniamin commented Mar 7, 2022

Din ce inteleg, intr-un loc de cazare pot sta mai multi refugiati. Asta inseamna ca in pagina spatiului de cazare, un host ar trebui sa poata alege carui refugiat sa ii lase review-ul.

@beniamin
Copy link
Contributor

beniamin commented Mar 7, 2022

sistemul de rating ar trebui sa fie in ambele directii: atat refugiatul sa poata da review unei cazari cat si gazda sa poata da review unui refugiat.

Propun ca in acest PR sa rezolvam doar parte de review de la un refugiat pentru o cazare, iar review-ul in cealalta directie sa il facem intr-un PR separat.

@beniamin
Copy link
Contributor

beniamin commented Mar 7, 2022

Box-ul de review pentru o cazare apare atat pentru refugiat cat si pentru gazda, dar bat ambele in acelasi endpoint de review al refugiatului pentru cazare. Ar trebui ca box-ul sa nu apara host-ului.

@cstns
Copy link
Contributor Author

cstns commented Mar 7, 2022

Box-ul de review pentru o cazare apare atat pentru refugiat cat si pentru gazda, dar bat ambele in acelasi endpoint de review al refugiatului pentru cazare. Ar trebui ca box-ul sa nu apara host-ului.

uu nice catch, fixez acum

@beniamin
Copy link
Contributor

beniamin commented Mar 7, 2022

Pare ca rating-ul nu se seteaza corect. Oricat stelute as selecta, valoarea trimisa in backend este tot 0.

Poti adauga ceva erori mai user friendly pentru validarile de pe campuriel de review? ai aici cateva detalii: https://laravel.com/docs/7.x/validation#customizing-the-error-messages

@cstns
Copy link
Contributor Author

cstns commented Mar 7, 2022

Pare ca rating-ul nu se seteaza corect. Oricat stelute as selecta, valoarea trimisa in backend este tot 0.

Poti adauga ceva erori mai user friendly pentru validarile de pe campuriel de review? ai aici cateva detalii: https://laravel.com/docs/7.x/validation#customizing-the-error-messages

Done 👍

Despre validari, picau ca nu se updata hidden input dupa upgrade-ul de jquery lib, mesajele de eroare erau cele default. Am avut intentia sa le modific dar am vazut ca existau deja traduceri pentru ele asa ca le-am lasat asa.

@beniamin
Copy link
Contributor

beniamin commented Mar 9, 2022

am deschis un PR in branch-ul tau cu niste modificari.

every allocated refugee may write a review for same location; make re…
@cstns
Copy link
Contributor Author

cstns commented Mar 9, 2022

L-am acceptat, ti-am dat access pe fork-ul meu

@@ -74,4 +77,15 @@ public function viewAccommodation(Accommodation $accommodation)
->with('otherFacilities', $otherFacilities)
->with('availabilityIntervals', $availabilityIntervals);
}

public function reviewAccommodation(AccommodationReviewRequest $request, Accommodation $accommodation): View
Copy link
Contributor

Choose a reason for hiding this comment

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

aici ar fi bine sa faci un double check ca userul curent chiar a fost alocat (probabil si ca a fost cazat la locatia) unde lasa review.

Copy link
Member

Choose a reason for hiding this comment

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

sounds like a good idea

@andreiio andreiio changed the base branch from development to main March 12, 2022 10:44
Copy link
Member

@andreiio andreiio left a comment

Choose a reason for hiding this comment

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

We're mostly there. Mai rămân de rezolvat înainte să facem merge:

  1. câteva todo comments prin cod;
  2. o sugestie de extra checks de la @beniamin;
  3. the merge conflict

Comment on lines +200 to +203
public function reviewedByUser(User $user): bool
{
return (bool)$this->reviews()->where(['user_id' => $user->id])->count();
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public function reviewedByUser(User $user): bool
{
return (bool)$this->reviews()->where(['user_id' => $user->id])->count();
}
public function isReviewedBy(User $user): bool
{
return $this->reviews()->where('user_id', $user->id)->exists();
}

</h6>
</div>

@if ($accommodation->reviewedByUser(auth()->user()))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
@if ($accommodation->reviewedByUser(auth()->user()))
@if ($accommodation->isReviewedBy(auth()->user()))

@@ -74,4 +77,15 @@ public function viewAccommodation(Accommodation $accommodation)
->with('otherFacilities', $otherFacilities)
->with('availabilityIntervals', $availabilityIntervals);
}

public function reviewAccommodation(AccommodationReviewRequest $request, Accommodation $accommodation): View
Copy link
Member

Choose a reason for hiding this comment

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

sounds like a good idea

Comment on lines +261 to +265
if ($count = $this->reviews()->count()) {
return $this->reviews()->sum('rating') / $count;
}

return 0;
Copy link
Member

Choose a reason for hiding this comment

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

aici putem avea un query în loc de două

Suggested change
if ($count = $this->reviews()->count()) {
return $this->reviews()->sum('rating') / $count;
}
return 0;
return $this->reviews()->avg('rating') ?? 0;

*/
public function authorize(): bool
{
return Auth::user()->isRefugee();
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return Auth::user()->isRefugee();
return $this->user()->isRefugee();

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.

[Must Have] Implement review option
4 participants