Skip to content
This repository was archived by the owner on Apr 15, 2024. It is now read-only.

Challenges#2

Merged
Alabate merged 41 commits intomasterfrom
challenges
Aug 6, 2018
Merged

Challenges#2
Alabate merged 41 commits intomasterfrom
challenges

Conversation

@nyc4m
Copy link
Copy Markdown

@nyc4m nyc4m commented Aug 4, 2018

DONE

  • Admin can add challenges, specifying
    • name
    • description
    • points
    • deadline
  • teams leader can validate challenges with a picture, provided they do not exceed the deadline
  • admins can see all the challenges validation and can
    • refuse it, providing a message
      * accept it, in that case the team win the points from the challenges
  • The entire team can check all the challenges they have handled, especially they can see if the challenge is
    • pending
    • validated
    • refused (with the message)
  • Admins can have an history of the validation, and potentially undo a validation

baptistePrunot and others added 30 commits June 13, 2018 21:35
as well as a page for the team leader in order for him to have a feedback on the challenges he sent for validation
Added Image lib to handle image resizing for challenges validation
@nyc4m nyc4m requested a review from Alabate August 4, 2018 14:27
* it is usefull in laravel 5.2, since you can't retrieve an array
* from the request directly, or I did not find a way :|
*/
private function challengeRequest2Array(ChallengeRequest $request):array {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Je connaissais pas l'utilisation de ces classes request, c'est pas mal !
Du coup sans l'utilisation de ces classes y'a le Request::only(), mais du coup je pense pas que tu peux l'appliquer là. Par contre je pense que ça serait mieux de mettre cette méthode dans la classe ChallengeRequest : ça permettrait de faire une simple méthode toArray(), bien plus logique que dans le controller, non ?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Effectivement, je vais changer ça

$challenge = $this->challengeRequest2Array($request);
Challenge::create($challenge);
$request->session()->flash('success', 'Défis ajouté');
return redirect("challenges/");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

  • Globalement la convention sur tout le projet (il me semble) c'est d'utiliser que des simple quotes pour les strings histoire d'être uniforme et de pas choisir au hasard à chaque fois. Sauf exception évidement en cas de besoin du \n par exemple. Et pour le html ça reste du double quote.
  • Ca serait mieux d'utiliser les nom des routes pour la redirection. J'ai vu que tu l'a fais sur les autres méthodes, c'est volontaire ?
  • Pour info, pour le flash, laravel integre des trucs assez sympa pour la redirection :
return Redirect::route('challenges.list')->withSuccess('Défis ajouté.');

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

  • Je cache pas que pour cette histoire de quote, j'ai un peu fait au hasard ^^'
  • Pour le redirection ça doit-être du code que j'ai écrit au début ou je savais pas pour le nom des routes, je vais changer
  • Je vais aussi changer pour le flash

Comment thread config/app.php

/*
* Additional libs
*/
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Tu as utilisé des tabulations alors que tout le projet utilise des indentations à 4 espaces. (Et c'est pas le seul endroit, il me semble)

"points" => $faker->numberBetween(1, 50),
"deadline" => $faker->datetime
];
});
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Gros 👍 pour avoir fait ça !

<li><a href="{{ route('dashboard.referrals.slides.branch') }}">Diapo Branche</a></li>
</ul>
</li>
<li class="dropdown">
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Pas visible depuis l'interface CE parce qu'il y a un @if (Auth::user()->isAdmin()) plus haut.

Copy link
Copy Markdown
Author

@nyc4m nyc4m Aug 5, 2018

Choose a reason for hiding this comment

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

Pour cette partie c'est bizarre parce que j'avais justement testé en me retirant les droits admin... je vais revérifier !

EDIT : my bad je corrige

<td>{{ $challenge->points }}</td>
<td>{{ $challenge->deadline }}</td>
<td>
<form action={{ route("challenges.delete", ["id"=>$challenge->id]) }} method="POST">
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

  • Il faudrait cacher les boutons modifier et supprimer sur l'interface CE
  • Sur le reste du site, les boutons dans les lignes d'une table sont généralement mis avec la classe btn-xs afin d'être plus petits

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

ok !

@Alabate Alabate merged commit 75db1f0 into master Aug 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants