Lehman black coding challenge

README.md

@@ -1,6 +1,6 @@
 # Coding Exercise
 
-Hello, _______________!
+Hello, Lehman Black!
 
 Below is a coding exercise that we believe will allow you to show off your amazing development skills!
 
@@ -76,6 +76,139 @@ The data will be displayed in a sortable table.
 
 You will need to determine the type of data in the CSV file based on the headers in the first row. Your program will output a list of Groups, and for each Group, a list of active People in that Group.
 
+### Validating the changes
+#### Running
+You will need an empty MySQL database to run the example. I used docker to spin up
+a server (I'm using version 5.7 due to the root account not working out of the box in
+newer container versions)
+
+```
+docker run --name breezedb -e MYSQL_ROOT_PASSWORD=root -e MYSQL_DATABASE=laravel -p3306:3306  mysql:5.7
+```
+
+I also had to install the php-sqlite3 module to get the tests to run.
+The installation of `yarn` was missing from my system too, so I had to use `npm`
+
+```
+npm install -g yarn
+```
+
+After that I could follow the set up instructions above without any issues.
+1. copy `.env.example` to .env and add database information
+2. run `composer install`
+3. run `yarn install`
+4. run `php artisan key:generate && php artisan migrate`
+4. start API using `php artisan serve`
+5. start react with `yarn start`
+
+#### Changes Implemented
+Added the API endpoint for `api/groups` and supports all the normal CRUD operations.
+
+The `group_name` column has been added to the `people` table. This can be any string
+since it is not checked with the `groups`. In production this should be a foreign key.
+This column is also displayed with the other `person` data in the React application.
+
+Automated testing has been added for the People and Groups APIs. They can be run with
+```
+php artisan test
+```
+
+`curl` can be used to manually check API endpoints. The available routes are:
+
+```
+# php artisan route:list
+
++--------+-----------+--------------------------+----------------+-----------------------------------------------+------------+
+| Domain | Method    | URI                      | Name           | Action                                        | Middleware |
++--------+-----------+--------------------------+----------------+-----------------------------------------------+------------+
+|        | GET|HEAD  | api/groups               | groups.index   | App\Http\Controllers\GroupsController@index   | api        |
+|        | POST      | api/groups               | groups.store   | App\Http\Controllers\GroupsController@store   | api        |
+|        | GET|HEAD  | api/groups/create        | groups.create  | App\Http\Controllers\GroupsController@create  | api        |
+|        | GET|HEAD  | api/groups/{group}       | groups.show    | App\Http\Controllers\GroupsController@show    | api        |
+|        | PUT|PATCH | api/groups/{group}       | groups.update  | App\Http\Controllers\GroupsController@update  | api        |
+|        | DELETE    | api/groups/{group}       | groups.destroy | App\Http\Controllers\GroupsController@destroy | api        |
+|        | GET|HEAD  | api/groups/{group}/edit  | groups.edit    | App\Http\Controllers\GroupsController@edit    | api        |
+|        | GET|HEAD  | api/people               | people.index   | App\Http\Controllers\PeopleController@index   | api        |
+|        | POST      | api/people               | people.store   | App\Http\Controllers\PeopleController@store   | api        |
+|        | GET|HEAD  | api/people/create        | people.create  | App\Http\Controllers\PeopleController@create  | api        |
+|        | GET|HEAD  | api/people/{person}      | people.show    | App\Http\Controllers\PeopleController@show    | api        |
+|        | PUT|PATCH | api/people/{person}      | people.update  | App\Http\Controllers\PeopleController@update  | api        |
+|        | DELETE    | api/people/{person}      | people.destroy | App\Http\Controllers\PeopleController@destroy | api        |
+|        | GET|HEAD  | api/people/{person}/edit | people.edit    | App\Http\Controllers\PeopleController@edit    | api        |
++--------+-----------+--------------------------+----------------+-----------------------------------------------+------------+
+```
+
+The React application now has a place at the top to upload CSV files. The type of
+resource is automatically detected. If the CSV file is not a recognized resource
+then nothing happens when you click the upload button (see the tasks remaining section)
+
+There is some console logs for when the upload finishes, but nothing displays on the screen.
+You must also refresh the page to view the data changes
+
+The CSV files do not depend on each other so you can upload them in any order.
+(benefit of using a string for the `person.group_name`. In production, I would
+create any missing groups or default missing groups to an empty string)
+
+Sample CSV files can be found in `tests/sample_data`.
+
+
+#### CSV Format
+People
+```
+id,first_name,last_name,email_address,status,group_name
+1,"Alex","Ortiz-Rosado","alex@breezechms.com",active,"Bible Study"
+2,"Jon","VerLee","jon@breezechms.com","archived","Bible Study"
+3,"Fred","Flintstone","fredflintstone@example.com","active","Bible Study"
+4,"Marie","Bourne","mbourne@example.com","active","Vacation"
+5,"Wilma","Flintstone","wilmaflinstone@example.com","active","Elders"
+```
+
+Groups
+```
+id,group_name
+1,Volunteers
+2,Elders
+3,"Bible Study"
+```
+
+#### Known Issues
+* **columns are not sortable. ...It's just a matter of adding the right `sortable` state as given in
+ https://react.semantic-ui.com/collections/table/#variations-sortable**
+* `id` is required, but does not quite work as expected. It is an autoincrementing
+column so new ID's may not match if you skip numbers or do not have the IDs in a sorted order
+* `jest` tests for ResultsList is broken due to updates. Should just need to update the expected
+data with the new JSX changes
+* have not checked for how large a CSV can be processed
+* uploading multiple files without refreshing didn't work during testing
+* The PapaParse JavaScript library can be tweaked to be more friendly, but is kind of strict
+* Some console errors due to 204 No Content responses from the API being handled improperly
+using the defaults. I have noticed issues with the following:
+    * Columns should not have spaces between them. (Bad: `id, group_name`, Good: `id,group_name` )
+    * PapaParse doesn't allow you set multiple quote characters, so I set it to use `"`
+
+#### Tasks Remaining
+* UI feedback
+    * Add dimmer to show files are uploading and prevent multiple uploads accidently
+    * add upload progress (PapaParse lets you have a callback for each row / chain state updates to the fetch promises)
+    * notify user when upload finishes
+    * show results of the upload (created, updated, errors)
+    * add link in upload results to show error details
+    * automatically refresh tables
+    * allow user to download sample files
+    * notify user if no file selected or bad file selected
+    * breeze demo upload uses popup for uploads, would style to match production conventions
+* better error handling
+* abstract `PeopleService` and `GroupService` into a factory. All that really changes is the URL,
+and the allowed fields
+* verify CSRF or other XSS protection beyond React's defaults are needed
+* If performance became an issue then the CSV parsing and API requests could be done in batches
+with some modifications to the API POST/PUT endpoints.
+* Update JS to match coding standards (current company does not use ES6+ so I'm a bit rusty)
+* Add missing code documentation
+
+
+
+
 ### Testing
 
 We love TDD! So we’d love to see tests for the API and ReactJS application. Write automated tests to verify your results and account for gotchas (handling different column orders, invalid id's in the People CSV file, etc..). Classify your tests as either unit, integration, ui, or acceptance, but it is not required to use every type.

app/Http/Controllers/GroupsController.php

@@ -0,0 +1,103 @@
+<?php
+
+namespace App\Http\Controllers;
+
+use Illuminate\Http\Request;
+use Illuminate\Validation\Rule;
+
+use App\Http\Resources\GroupsCollection;
+use App\Http\Resources\GroupResource;
+use App\Models\Group;
+
+class GroupsController extends Controller
+{
+    /**
+     * Display a listing of the resource.
+     *
+     * @return \Illuminate\Http\Response
+     */
+    public function index()
+    {
+        return new GroupsCollection(Group::all());
+    }
+
+    /**
+     * Show the form for creating a new resource.
+     *
+     * @return \Illuminate\Http\Response
+     */
+    public function create()
+    {
+        //
+    }
+
+    /**
+     * Store a newly created resource in storage.
+     *
+     * @param  \Illuminate\Http\Request  $request
+     * @return \Illuminate\Http\Response
+     */
+    public function store(Request $request)
+    {
+        $request->validate([
+            'group_name'    => 'required|max:255',
+        ]);
+
+        $group = Group::create($request->all());
+
+        return (new GroupResource($group))
+            ->response()
+            ->setStatusCode(201);
+    }
+
+    /**
+     * Display the specified resource.
+     *
+     * @param  int  $id
+     * @return \Illuminate\Http\Response
+     */
+    public function show($id)
+    {
+        return new GroupResource(Group::findOrFail($id));
+    }
+
+    /**
+     * Show the form for editing the specified resource.
+     *
+     * @param  int  $id
+     * @return \Illuminate\Http\Response
+     */
+    public function edit($id)
+    {
+        //
+    }
+
+    /**
+     * Update the specified resource in storage.
+     *
+     * @param  \Illuminate\Http\Request  $request
+     * @param  int  $id
+     * @return \Illuminate\Http\Response
+     */
+    public function update(Request $request, $id)
+    {
+        $group = Group::findOrFail($id);
+        $group->update($request->all());
+
+        return response()->json(null, 204);
+    }
+
+    /**
+     * Remove the specified resource from storage.
+     *
+     * @param  int  $id
+     * @return \Illuminate\Http\Response
+     */
+    public function destroy($id)
+    {
+        $group = Group::findOrFail($id);
+        $group->delete();
+
+        return response()->json(null, 204);
+    }
+}

app/Http/Resources/GroupResource.php

@@ -0,0 +1,24 @@
+<?php
+
+namespace App\Http\Resources;
+
+use Illuminate\Http\Resources\Json\JsonResource;
+
+class GroupResource extends JsonResource
+{
+    /**
+     * Transform the resource into an array.
+     *
+     * @param \Illuminate\Http\Request $request
+     * @return array
+     */
+    public function toArray($request)
+    {
+        return [
+            'id' => $this->id,
+            'group_name' => $this->group_name,
+            'created_at'    => $this->created_at,
+            'updated_at'    => $this->updated_at,
+        ];
+    }
+}

app/Http/Resources/GroupsCollection.php

@@ -0,0 +1,21 @@
+<?php
+
+namespace App\Http\Resources;
+
+use Illuminate\Http\Resources\Json\ResourceCollection;
+
+class GroupsCollection extends ResourceCollection
+{
+    /**
+     * Transform the resource collection into an array.
+     *
+     * @param \Illuminate\Http\Request $request
+     * @return array
+     */
+    public function toArray($request)
+    {
+        return [
+            'data' => $this->collection
+        ];
+    }
+}

app/Http/Resources/PersonResource.php

@@ -20,6 +20,7 @@ public function toArray($request)
          'last_name'     => $this->last_name,
          'email_address' => $this->email_address,
          'status'        => $this->status,
+         'group_name'    => $this->group_name,
          'created_at'    => $this->created_at,
          'updated_at'    => $this->updated_at,
      ];

app/Models/Group.php

@@ -0,0 +1,12 @@
+<?php
+
+namespace App\Models;
+
+use Illuminate\Database\Eloquent\Model;
+
+class Group extends Model
+{
+    protected $fillable = [
+        'group_name',
+    ];
+}

app/Models/Person.php

@@ -10,6 +10,7 @@ class Person extends Model
         'first_name',
         'last_name',
         'email_address',
-        'status'
+        'status',
+        'group_name',
     ];
 }

composer.json

@@ -9,8 +9,9 @@
     "license": "MIT",
     "require": {
         "php": "^7.2.5",
-        "fruitcake/laravel-cors": "^2.0",
+        "doctrine/dbal": "^2.10",
         "fideloper/proxy": "^4.0",
+        "fruitcake/laravel-cors": "^2.0",
         "laravel/framework": "^7.0",
         "laravel/tinker": "^2.0"
     },

database/factories/GroupFactory.php

@@ -0,0 +1,12 @@
+<?php
+
+/** @var \Illuminate\Database\Eloquent\Factory $factory */
+
+use App\Models\Group;
+use Faker\Generator as Faker;
+
+$factory->define(Group::class, function (Faker $faker) {
+    return [
+        'group_name' => $faker->name,
+    ];
+});

database/factories/PersonFactory.php

@@ -10,6 +10,7 @@
         'first_name' => $faker->firstName,
         'last_name' => $faker->lastName,
         'email_address' => $faker->email,
-        'status' => (bool)random_int(0, 1) ? 'active' : 'archived'
+        'status' => (bool)random_int(0, 1) ? 'active' : 'archived',
+        'group_name' => $faker->name,
     ];
 });

database/migrations/2020_06_28_222744_create_groups_table.php

@@ -0,0 +1,34 @@
+<?php
+
+use Illuminate\Database\Migrations\Migration;
+use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Support\Facades\Schema;
+
+class CreateGroupsTable extends Migration
+{
+    /**
+     * Run the migrations.
+     *
+     * @return void
+     */
+    public function up()
+    {
+        Schema::table('groups', function (Blueprint $table) {
+            Schema::create('groups', function (Blueprint $table) {
+                $table->bigIncrements('id');
+                $table->string('group_name');
+                $table->timestamps();
+            });
+        });
+    }
+
+    /**
+     * Reverse the migrations.
+     *
+     * @return void
+     */
+    public function down()
+    {
+        Schema::dropIfExists('groups');
+    }
+}