Codeigniter Controller for work orders

Question

I'm working on a project and one of my controllers is getting quite large and hard to maintain or add to. I'm having trouble deciding what belongs in the controller and what I can move to the model. What can I change to improve this?

class Job_update extends MY_Controller
{
function __construct()
{
 parent::__construct();
 $this->load->helper('form');
 $this->load->library('email');
 $this->load->helper('html');
 $this->load->model('jobs_model');
}
/**
 * Reassigns a tradie to the work order sending appropriate emails out
 * @param int $job_id 
 */
public function reassign_tradie($job_id)
{
 $this->load->model('send_emails_model');
 $this->load->model('tradie_model');
 $this->load->model('notes_model');
 //get tradie ids via get or post
 if ($this->input->get("tradie")) {
 $tradie_id = $this->input->get("tradie");
 $prev_tradie = $this->input->get("prev_tradie");
 } else {
 $tradie_id = $this->input->post("tradie");
 $prev_tradie = $this->input->post("prev_tradie");
 }
 if ($tradie_id == $prev_tradie) {
 $this->session->set_flashdata('error', "That tradesman is already assigned to this job");
 redirect('job_update/job_id/'.$job_id);
 return;
 }
 //retrieve data for both tradies
 $data = $this->tradie_model->get($tradie_id); //new tradie
 $prev_tradie_data = $this->tradie_model->get($prev_tradie);//prev tradie
 //update the status to pending and set the new tradie
 $this->jobs_model->update($job_id, array(
 "status" => "Pending",
 "tradie" => $tradie_id,
 "job_tradie_response" => 0
 ));
 //add a note to the job documenting the change
 $job_note_message = "Tradesman has been changed from " . $prev_tradie_data->tradie_firstname .
 " to ". $data->tradie_firstname . " Reason: " . $this->input->post("reason");
 $this->notes_model->insert($job_id, $job_note_message);
 //select the new job data
 $job_data = $this->jobs_model->with_site_location()->with_tradie()->get($job_id);
 //send the tradie an email of the work order
 $tradie_email = $this->jobs_model->build_tradie_email($job_id, $job_data);
 $this->send_emails_model->insert($tradie_email);
 //if the prev tradie didn't decline the job we need to send a cancellation email
 if ($job_data->status != "Declined") {
 $this->load->model('tradie_model');
 $tradie_data = $this->tradie_model->get($prev_tradie);
 $cancelled_email = $this->jobs_model->build_job_cancelled_email($job_id, $tradie_data);
 $this->send_emails_model->insert($cancelled_email);
 }
 //redirect
 $this->session->set_flashdata('msg', "Tradesman has been successfully changed");
 redirect('job_update/job_id/'.$job_id);
}
/**
 * Resends the work order to the assigned plumber also updates the status to pending
 * @param int $job_id 
 */
public function resend_work_order($job_id)
{
 $this->load->model('notes_model');
 $this->load->model('send_emails_model');
 //updates status of job
 $job_update = array("status" => "Pending", "job_tradie_response" => 0);
 $this->jobs_model->update($job_update, $job_id);
 //Sends email to tradesman
 $job_data = $this->jobs_model->with_site_location()->with_tradie()->get($job_id);
 $result = $this->jobs_model->build_tradie_email($job_id, $job_data);
 $this->send_emails_model->insert($result);
 //addes note to system talking about change
 $data = array(
 'note' => "Job has been resent to the plumber. Status changed to Pending",
 'notes_job_id' => $job_id,
 'datestamp' => date("Y-m-d H:i:s"),
 'user_id' => $this->session->userdata('username')
 );
 $this->notes_model->insert($data);
 $this->session->set_flashdata('msg', 'Resent Work Order to '. $job_data->tradie->tradie_firstname);
 redirect('job_update/job_id/'.$job_id);
}
/**
 * Inserts a new note from post data
 * @param int $job_id 
 */
public function job_add_note($job_id)
{
 $this->_add_note($job_id, $this->input->post('job_notes'));
 $this->session->set_flashdata('msg', 'Job Note Added Successfully');
 redirect('job_update/job_id/'.$job_id);
}
/**
 * Loads the photos page for a certain job
 * @param int $job_id
 */
public function photos($job_id)
{
 $data = array('job_id' => $job_id);
 $this->load->view('job_photos_view', $data);
}
public function set_flash_for_bookin($job_id)
{
 $this->session->set_flashdata('msg', 'Date/Time has been successfully set');
 redirect('job_update/job_id/'.$job_id);
}
/**
 * Saves the work order details (checkboxes, job_description)
 * @param int $job_id 
 */
public function submit_work_order($job_id)
{
 $data = $this->update_checkboxes($job_id); //get all checkbox data
 $this->jobs_model->update($data, $job_id); //save to db
 $this->session->set_flashdata('msg', 'Work Order Updated Successfully');
 redirect('job_update/job_id/'.$job_id);
}
/**
 * Resends the customer confirmation email
 * @param string (email) $c_email 
 * @param int $job_id 
 */
public function resend_confirmation_email($c_email, $job_id)
{
 $this->load->model('send_emails_model');
 $this->job_update_model->update_send_email(array("email_sent" => 0), urldecode($c_email));
 $this->session->set_flashdata('msg', 'Email Successfully Sent');
 redirect('job_update/job_id/'.$job_id);
}
/**
 * Sets the date for a checkbox
 * @param string $post_data 
 * @param date $current_date_in_db 
 * @param bool (0,1) $is_checked 
 * @return date string format YYYY-MM-DD
 */
private function checkbox_helper($post_data, $current_date_in_db, $is_checked) {
 if($post_data == false)
 return "0000-00-00";
 else if($post_data != false && $is_checked === "0")
 return date("Y-m-d");
 else
 return $current_date_in_db;
}
/**
 * returns an array with checkbox data
 * @param int $job_id 
 * @return array 
 */
public function update_checkboxes($job_id) {
 $job_data = $this->jobs_model->get($job_id);
 $current_date = date('Y-m-d');
 $current_time = date('H:i:s');
 //if 1 box is checked if 0 box is not checked
 $data = array(
 'box_customer_called' => ($this->input->post('checkbox1') == false ? '0' : '1'),
 'box_customer_called_date' => $this->checkbox_helper($this->input->post('checkbox1'), $job_data->box_customer_called_date, $job_data->box_customer_called),
 'box_photos_reviewed' => ($this->input->post('checkbox2') == false ? '0' : '1'),
 'box_photos_received_date' => $this->checkbox_helper($this->input->post('checkbox2'), $job_data->box_photos_received_date, $job_data->box_photos_reviewed),
 'box_photos_received' => ($this->input->post('checkbox6') == false ? '0' : '1'),
 'box_photos_received_date' => $this->checkbox_helper($this->input->post('checkbox6'), $job_data->box_photos_received_date, $job_data->box_photos_received),
 'box_completed' => ($this->input->post('checkbox7') == false ? '0' : '1'),
 'box_completed_date' => $this->checkbox_helper($this->input->post('checkbox7'), $job_data->box_completed_date, $job_data->box_completed),
 'box_completion_certificate' => ($this->input->post('checkbox3') == false ? '0' : '1'),
 'box_completion_certificate_date' => $this->checkbox_helper($this->input->post('checkbox3'), $job_data->box_completion_certificate_date, $job_data->box_completion_certificate),
 'box_invoiced' => ($this->input->post('checkbox4') == false ? '0' : '1'),
 'box_invoiced_date' => $this->checkbox_helper($this->input->post('checkbox4'), $job_data->box_invoiced_date, $job_data->box_invoiced),
 'box_warranty' => ($this->input->post('checkbox5') == false ? '0' : '1'),
 'box_warranty_date' => $this->checkbox_helper($this->input->post('checkbox5'), $job_data->box_warranty_date, $job_data->box_warranty),
 'job_description' => $this->input->post('job_description_new'),
 );
 //if the job was completed and is now unticked change the status
 if($job_data->box_completed == 0 && $this->input->post('checkbox7') == "Completed")
 $data['status'] = "Completed";
 if($job_data->box_completed == 1 && $this->input->post('checkbox7') != "Completed")
 $data['status'] = "Accepted";
 return $data;
}
/**
 * Inserts an email into the send que
 * @param string $message 
 * @param string $email 
 * @param string $name 
 * @param string $email_subject 
 */
public function email($message, $email, $name, $email_subject)
{
 $this->load->model('send_emails_model');
 $data = array('message' => $message,
 'email' => $email,
 'name' => $name,
 'email_subject' => $email_subject);
 $this->send_emails_model->insert($data);
}
/**
 * Uploads a photo to the job directotry
 * @param int $job_id 
 */
public function upload_images($job_id)
{
 $name_array = array();
 if (!is_dir('assets/job_photos/'.add_prefix($job_id))) {//create directory is it doesn't exist
 mkdir('./assets/job_photos/'.add_prefix($job_id), 0777, true);
 $dir_exist = false; // dir not exist
 }
 $count = count($_FILES['userfile']['size']);
 foreach ($_FILES as $key => $value) {
 for ($s=0; $s<=$count-1; $s++) {
 $_FILES['userfile']['name']=$value['name'][$s];
 $_FILES['userfile']['type'] = $value['type'][$s];
 $_FILES['userfile']['tmp_name'] = $value['tmp_name'][$s];
 $_FILES['userfile']['error'] = $value['error'][$s];
 $_FILES['userfile']['size'] = $value['size'][$s];
 $config['upload_path'] = './assets/job_photos/'.add_prefix($job_id).'/';
 $config['allowed_types'] = 'gif|jpg|png|jpeg|pdf|PNG|JPG|JPEG';
 $this->load->library('upload', $config);
 if (! $this->upload->do_upload()) {
 $this->session->set_flashdata('error', $this->upload->display_errors());
 } else {
 $this->session->set_flashdata('msg', 'Successully Uploaded');
 }
 }
 }
 redirect(site_url('job_update/job_id/'.$job_id));
}
public function job_id($job_id) 
{
 $this->load->model('user_model');
 $this->load->model('tradie_model');
 $this->load->model('send_emails_model');
 $this->load->model('notes_model');
 //gather all data required for display
 $job_data = $this->jobs_model->with_site_location()->with_tradie()->with_job_notes()->get($job_id);
 $all_users['users'] = $this->user_model->select_all_users();
 $tradies['tradies'] = $this->tradie_model->get_all();
 $email = $this->send_emails_model->where('email', $job_data->site_location->tenant_email)->get();
 //if job isn't found job data will be empty so we load a blank page and return
 if (empty($job_data)) {
 $this->headerlib->load_header();
 return;
 }
 //change all 1's to checked for view
 foreach ($job_data as $key => &$value) {
 //filter thourgh all table coloumns to find the boxes coloumns
 if(strpos($key, 'box') !== false && strpos($key, 'date') === false) {
 if($value == 1)
 $value = "checked";
 }
 }
 $data = array_merge((array)$job_data, (array)$all_users, (array)$job_emails, (array)$tradies);
 $this->headerlib->load_header();
 $this->footerlib->load_footer();
 $this->load->view('job_update_view', $data);
}
/**
 * Deletes a job
 * @param int $job_id job_id you wish to delete
 */
public function delete_job($job_id)
{
 $this->load->model('send_emails_model');
 $this->load->model('tradie_model');
 if($this->session->userdata('user_team_id') != 40)
 {
 $this->session->set_flashdata('error', 'You do not have permission to delete this job');
 redirect($_SERVER['HTTP_REFERER']);
 return;
 }
 $data = $this->jobs_model->with_tradie()->get($job_id);
 $tradie_data = $this->tradie_model->get($data->tradie->tradie_id);
 $cancelled_email = $this->jobs_model->build_job_cancelled_email($job_id, $tradie_data);
 $this->send_emails_model->insert($cancelled_email);
 if ($this->jobs_model->delete($job_id)) {
 $this->session->set_flashdata('msg', 'Job has been successfully deleted');
 } else {
 $this->session->set_flashdata('error', 'Something went wrong, contact an administrator');
 }
 redirect($_SERVER['HTTP_REFERER']);
}
}

Thanks for the help!

• Sᴀᴍ Onᴇᴌᴀ: Welcome to Code Review! I changed the title so that it describes what the code does per site goals: "State what your code does in your title, not your main concerns about it.". Feel free to edit and give it a different title if there is something more appropriate.

Answer 1

• Models are for data models, you shold not be using them to do things like sending emails, you should be using libraries for that kind of task instead, and have the library insert a record into the model if required.
• Your $data array in update_checkboxes() should be generated in a model, since you're creating a data model. I would accomplish this by adding a method to the jobs model and passing it the user input, then have it return that array/model, eg $data = $this->jobs_model->get_data_array($this->input);
• You shouldn't be assigning things to the $_FILES superglobal (or any superglobals) you should pass the required $values array to the library directly. Also, your $config variables don't change in the loop so why re-assign them on every iteration.
• You could use array_map to change all the 1s to checked, although that's probably the type of thing that should be done in a model.

• rtzcoder: Thanks this will help a lot! Do you think the long functions like reassign_tradie($job_id) should be broken up into smaller functions?
• I wrestled a bear once.: @rtzcoder ideally all functions would be short but by the same token, you shouldn't make your functions overly short just because some convention says you should. controllers are usually the exception. some of the CI controllers we have at my job are extremely long and there's no obvious semantic way to shorten them. I think your method is fine and I can't see an obvious way to shorten it.