Skip to content

Packagify the TaskFighter part of the application#1

Open
RStrydom wants to merge 1 commit intomasterfrom
feature/taskfighter-package
Open

Packagify the TaskFighter part of the application#1
RStrydom wants to merge 1 commit intomasterfrom
feature/taskfighter-package

Conversation

@RStrydom
Copy link
Owner

Packigifying the TaskFighter App

Copy link

@JustinShift JustinShift left a comment

Choose a reason for hiding this comment

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

The code that is in your /packages dir is missing from the PR

Comment on lines +38 to +41
public function boot()
{
//
}

Choose a reason for hiding this comment

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

This is a problem, I think you need to call parent or comment out the stub method.

| Here is where you can register API routes for TaskFighter.
|
| Assumed:
| - prefix = api/tasks

Choose a reason for hiding this comment

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

This is a lie check service provider

Comment on lines +25 to +27
$taskFighter = new \App\Models\Task($task->name, $task->priority, $task->due_in);
$taskFighter->tick();
DB::update("update tasks set priority = '{$taskFighter->priority}', due_in = '{$taskFighter->due_in}' where id = '{$task->id}'");

Choose a reason for hiding this comment

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

You creating a model and then doing the update in plain SQL?

*/


Route::get('list/tick', function (Response $response) {

Choose a reason for hiding this comment

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

Why is this not in a controller, why inline and why DB::table instead of Task:all().

Comment on lines +23 to +27
$tasks = DB::table('tasks')->select('*')->get();
foreach ($tasks as $task) {
$taskFighter = new \App\Models\Task($task->name, $task->priority, $task->due_in);
$taskFighter->tick();
DB::update("update tasks set priority = '{$taskFighter->priority}', due_in = '{$taskFighter->due_in}' where id = '{$task->id}'");

Choose a reason for hiding this comment

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

This needs a rewrite. I suggest having a look at Collection higher order messages support.

I think you can do this with:

Task:all()->each->tick()->each->save();

If that does not work and you want to do it in one loop you can do this.

Task::all()->each(function(Task $task) {
  $task->tick();
  $task->save();
});

Rer: https://laravel.com/docs/5.5/collections#higher-order-messages

"psr-4": {
"App\\": "app/"
"App\\": "app/",
"": "app/Packages/"

Choose a reason for hiding this comment

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

🎉

Comment on lines -23 to -32
Route::get('list/tick', function (Response $response) {
$tasks = DB::table('tasks')->select('*')->get();
foreach ($tasks as $task) {
$taskFighter = new \App\Task($task->name, $task->priority, $task->due_in);
$taskFighter->tick();
DB::update("update tasks set priority = '{$taskFighter->priority}', due_in = '{$taskFighter->due_in}' where id = '{$task->id}'");
}

return $response->setStatusCode(204);
});

Choose a reason for hiding this comment

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

This is definitely a skills test honey pot.

Comment on lines +1 to +18
<?php

namespace Tests\Unit;

use PHPUnit\Framework\TestCase;

class TaskTest extends TestCase
{
/**
* A basic unit test example.
*
* @return void
*/
public function testExample()
{
$this->assertTrue(true);
}
}

Choose a reason for hiding this comment

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

😕

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.

2 participants