v3 release

[hichxm]

DaybydayCRM V3

@Bottelet and I discussed possible v3. We reported a lot of concerns on v2 (optimization, security, code standard)

The purpose of this discussion is to bring up all the existing problems on v2 so as not to redo them on v3.

We have two solutions :

• Refactoring actual code for the v3
• Create from scratch the v3

Thank you

[hichxm]

Need memory optimizations in issue #245 and @Bottelet said this #245 (comment)

I think is not the only optimizations' problem on the CRM.

[hichxm]

For the v3, I think it is better if we use the classic Laravel storage system ?

We have complicated PR like #194

[hichxm]

In the repositories, we not need to pass request as argument, but only data needed.

Example, this file (app/Repositories/Role/RoleRepository.php) :

    /**
     * @param $requestData
     */
    public function create($requestData)
    {
        $roleName = $requestData->name;
        $roleDescription = $requestData->description;
        Role::create([
            'name' => strtolower($roleName),
            'display_name' => ucfirst($roleName),
            'description' => $roleDescription
        ]);
    }

to this :

    /**
     * @param $name
     * @param $description
     */
    public function create($name, $description)
    {
        Role::create([
            'name' => strtolower($name),
            'display_name' => ucfirst($name),
            'description' => $description
        ]);
    }

[hichxm]

We have this following code, but we have too RoleRepository. Why is not used here ?

DaybydayCRM/app/Http/Controllers/RolesController.php

Lines 95 to 111 in 8b9c7e9

	/**
	* @param StoreRoleRequest $request
	* @return mixed
	*/
	public function store(StoreRoleRequest $request)
	{
	$roleName = $request->name;
	$roleDescription = $request->description;
	Role::create([
	'external_id' => Uuid::uuid4()->toString(),
	'name' => strtolower($roleName),
	'display_name' => ucfirst($roleName),
	'description' => $roleDescription
	]);
	Session()->flash('flash_message', __('Role created'));
	return view('roles.index');
	}

[hichxm]

Same for this :

DaybydayCRM/app/Http/Controllers/RolesController.php

Lines 117 to 132 in 8b9c7e9

	public function destroy($external_id)
	{
	$role = Role::where('external_id', $external_id)->first();
	if (!$role->users->isEmpty()) {
	Session::flash('flash_message_warning', __("Can't delete role with users, please remove users"));
	return redirect()->route('roles.index');
	}
	if ($role->name !== Role::ADMIN_ROLE && $role->name !== Role::OWNER_ROLE) {
	$role->delete();
	} else {
	Session()->flash('flash_message_warning', __('Can not delete role'));
	return redirect()->route('roles.index');
	}
	Session()->flash('flash_message', __('Role deleted'));
	return redirect()->route('roles.index');
	}

[hichxm]

Many time repositories class not used

[hichxm]

Using PHP 8 ?

[hichxm]

Source : https://kinsta.com/fr/blog/comparaison-php/#laravel-8210

[hichxm]

Normally, the redirection to the OAuth connection link should be done at the backend level and not at the frontend level.

We have this :

@if($filesystem_integration)
        <p>
            {{ __('Connected with') }} {{ class_basename($filesystem_integration->name) }}
        </p>
        @endif
        <div class="col-sm-4 movedown">
        <img src="imagesIntegration/dropbox-logo.svg" width="60%" align="center" alt=""> <br>
        @if($filesystem_integration && $filesystem_integration->name == \App\Services\Storage\Dropbox::class)
            <form action="{{route('integration.revoke-access')}}" method="POST">
                                    {{ csrf_field() }}
                    <input type="submit" value="Unlink" class="btn btn-warning movedown">
            </form>
        @else
            <a href="{{$dropbox_auth_url}}"><button {{$filesystem_integration ? 'disabled' : ''}} class="btn btn-md btn-brand movedown">Link Dropbox</button></a>
        @endif
        </div>
        <div class="col-sm-4 movedown">
        <img src="imagesIntegration/google-drive-logo.png" width="70%" align="center" alt=""> <br>
        @if($filesystem_integration && $filesystem_integration->name == \App\Services\Storage\GoogleDrive::class)
             <form action="{{route('integration.revoke-access')}}" method="POST">
                    {{ csrf_field() }}
                    <input type="submit" value="Unlink" class="btn btn-warning movedown">
            </form>

        @else
            <a href="{{$google_drive_auth_url}}"><button {{$filesystem_integration ? 'disabled' : ''}} class="btn btn-md btn-brand movedown">Link Google Drive</button></a>
        @endif

And above all, it's not dynamic

[hichxm]

File : app/Repositories/Format/GetDateFormat.php

On the cited file, we create an additional process to display the date for different integration (for moment.js, for carbon ...).
The best method would be to change the date to ISO 8601 format. And then parse it at the frontend or backend level with moment.js, carbon or whatever.

[hichxm]

Security problem :
No validation rule .... We can use same email for user and have error "duplicate email entry SQL" or more problem.

DaybydayCRM/app/Http/Controllers/UsersController.php

Lines 171 to 208 in 8b9c7e9

	/**
	* @param StoreUserRequest $userRequest
	* @return mixed
	*/
	public function store(StoreUserRequest $request)
	{
	$settings = Setting::first();
	if (User::count() >= $settings->max_users) {
	Session::flash('flash_message_warning', __('Max number of users reached'));
	return redirect()->back();
	}
	$path = null;
	if ($request->hasFile('image_path')) {
	$file = $request->file('image_path');
	$filename = str_random(8) . '_' . $file->getClientOriginalName() ;
	$path = Storage::put($settings->external_id, $file);
	}
	$user = new User();
	$user->name = $request->name;
	$user->external_id = Uuid::uuid4()->toString();
	$user->email = $request->email;
	$user->address = $request->address;
	$user->primary_number = $request->primary_number;
	$user->secondary_number = $request->secondary_number;
	$user->password = bcrypt($request->password);
	$user->image_path = $path;
	$user->language = $request->language == "dk" ?: "en";
	$user->save();
	$user->roles()->attach($request->roles);
	$user->department()->attach($request->departments);
	$user->save();
	Session::flash('flash_message', __('User successfully added'));
	return redirect()->route('users.index');
	}

[hichxm]

Sorry, I don't see App\Http\Requests\User\StoreUserRequest class. But I already see no request validation before storing or updating data.

[hichxm]

No input validation here :

DaybydayCRM/app/Http/Controllers/ProductsController.php

Lines 23 to 46 in 8b9c7e9

	public function update(Request $request, $external_id = null)
	{
	if (!auth()->user()->can('product-edit')) {
	session()->flash('flash_message_warning', __('You do not have sufficient privileges for this action'));
	return redirect()->back();
	}
	if($external_id) {
	$product = Product::whereExternalId($external_id)->firstOrFail();
	} else {
	$product = Product::make();
	$product->external_id = Uuid::uuid4()->toString();
	}
	$product->name = $request->name;
	$product->description = $request->description;
	$product->default_type = $request->type;
	$product->price = $request->price * 100;
	$product->number = $request->product_number;
	$product->save();
	return redirect()->back();
	}

[hichxm]

Better explicit column or property. For example :

DaybydayCRM/app/Http/Controllers/ProductsController.php

Line 40 in 8b9c7e9

$product->price = $request->price * 100;

to :

 $product->price_in_cents = $request->price * 100;