1

I need to make request on some CRM api in my controller. For making this I have pretty big method. It's look like ugly. I know that there are some "Services" and to put additional code into Service is a good way. But I don't know what is this. Is it a custom classes into app folder? Or maybe it's Service-providers? I have read service-providers documentation and I'm not sure that service-providers is suitable for this. Here is my code:

<?php

namespace App\Http\Controllers;

use App\User;
use App\UserInfo;
use Validator;
use Illuminate\Http\Request;

class UserController extends Controller
{
/**
 * Display a listing of the resource.
 *
 * @return \Illuminate\Http\Response
 */
public function index(Request $request)
{
    $users = User::with('info')
            ->paginate(20);

    $users->withPath(DIRECTORY_SEPARATOR . $request->path() .DIRECTORY_SEPARATOR);

    return response()->json($users)->setEncodingOptions(JSON_UNESCAPED_UNICODE);
}

/**
 * Store a newly created resource in storage.
 *
 * @param  \Illuminate\Http\Request  $request
 * @return \Illuminate\Http\Response
 */
public function store(Request $request)
{
    $data = $request->json()->all();
    $rules = [
            'name' => 'required',
            'phone' => 'required|unique:users'
    ];
    $validator = Validator::make($data, $rules);
    if ($validator->fails()) return response()->json(['errors'=>$validator->errors()]);

        $user = new User();
        $user->name = request('name');
        $user->phone = request('phone');
        $user_info_obj = $this->storeUserInfo();
        if($user_info_obj === null){
            return response('Impassible to define user geo data', 400);
        }
        $user->info_id = $user_info_obj->id;
        $user->save();

        $this->makeAMOLead($user->name,
            $user->phone,
            $user_info_obj->user_agent,
            $user_info_obj->city,
            $user_info_obj->country);

        return response()->json(['success' => 'User created successfully']);
}

public function storeUserInfo()
{
    $ip = request()->ip();

    $reader = new \GeoIp2\Database\Reader('../resources/geo-lite2-city_20180807/GeoLite2-City.mmdb');

    try {
        $record = $reader->city($ip);
    }
    catch (\Throwable $e){
        // Code bellow is for testing on localhost, Because of maybe exception instead of geo obj on localhost.
        $info = new UserInfo();
        $info->ip = '127.0.0.1';
        $info->city = 'Some city';
        $info->country = 'Some country';
        $info->country_code = 'Some code';
        $info->continent = 'Some continent';
        $info->continent_code = 'no';
        $info->user_agent = 'User agent';

        $info->save();

        return $info;

        //return null;
    }

    $city = $record->city->names['ru'];
    $continent = $record->continent->names['ru'];
    $continent_code = $record->continent->code;
    $country = $record->country->names['ru'];
    $country_code = $record->country->isoCode;
    $user_agent = \request()->userAgent();

    $info = new UserInfo();
    $info->ip = $ip;
    $info->city = $city;
    $info->country = $country;
    $info->country_code = $country_code;
    $info->continent = $continent;
    $info->continent_code = $continent_code;
    $info->user_agent = $user_agent;

    $info->save();

    return $info;
}

private function makeAMOLead($name, $phone, $userAgent, $city, $country)
{
    $domain = env('AMO_DOMAIN');
    $login = env('AMO_LOGIN');
    $hash = env('AMO_HASH');

    try {
        $credentials = new \ddlzz\AmoAPI\CredentialsManager($domain, $login, $hash);

        $settings = new \ddlzz\AmoAPI\SettingsStorage();
        $settings->setCookiePath(env('AMO_COOKIE_FILE_PATH'));

        $request = \ddlzz\AmoAPI\ClientFactory::create($credentials, $settings);

        $lead = new \ddlzz\AmoAPI\Model\Amo\Lead();

        $lead['name'] =  $name;

        if(env('AMO_PIPELINE_ID', null)){
            $lead['pipeline_id'] = intval(env('AMO_PIPELINE_ID'));
        }

        $lead['name'] = 'New pickup user ' . $name;

        $lead['custom_fields'] = [
            [
                'id' => env('AMO_NAME_FIELD_ID'),
                'values' => [
                   ['value' => $name],
                ]
            ],
            [
                'id' => env('AMO_USER_AGENT_FIELD_ID'),
                'values' => [
                    ['value' => $userAgent]
                ]
            ],
            [
                'id' => env('AMO_CITY_FIELD_ID'),
                'values' => [
                    ['value' => $city]
                ]
            ],
            [
                'id' => env('AMO_COUNTRY_FIELD_ID'),
                'values' => [
                    ['value' => $country]
                ]
            ],

        ];

        $lead['created_at'] = time();
        $result = $request->add($lead);

        $pipelineId = json_decode($result)->_embedded->items{0}->id;

        // create contact
        $contact = new \ddlzz\AmoAPI\Model\Amo\Contact();
        $contact['name'] = $name;
        $contact['created_at'] = time();
        $contact['leads_id'] = "$pipelineId";
       // dd($request->accountInfo(), true); // Call this, if you need to know ids of default fields (like phone, or position)

        $contact['custom_fields'] = [
            [
                'id' => env('AMO_CONTACT_PHONE_ID'),
                'values' => [
                    [
                        'value' => $phone,
                        'enum' => 'MOB',
                    ],
                ]
            ],
        ];

        $result = $request->add($contact);

    } catch (Exception $e) {
        echo response()->json(['error' => $e->getFile() . ': ' . $e->getMessage()]);
    }
}

}

Look on the makeAMOLead. This is big method in my controller and this is not ok for controller conception.

4
  • Please include code, not images of code. Commented Sep 13, 2018 at 10:09
  • Ok, It will be code instead of picture. Commented Sep 13, 2018 at 10:14
  • 1
    This might be better suited for Code Review. Commented Sep 13, 2018 at 10:15
  • Thank you, I will try this resource. Commented Sep 13, 2018 at 10:20

1 Answer 1

1

Please use repository pattern to split all the communication between the application and your data source. and call the repository functions inside your controller. It is good practice. Here is an article you can understand about that

Example:

Your functions can be separate from controller to repository.

storeUserInfo

makeAMOLeadin

Move your functions an repository and call them into your controller.

Sign up to request clarification or add additional context in comments.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.