1
\$\begingroup\$

I am working on a basic blog application with Codeigniter 3.1.8 and Bootstrap 4.

The application has users (authors) and posts (articles). Every article:

  • has an author,
  • belongs to a category,
  • has a default (generic) main image that is displayed unless the author assigns a specific image to it.

The structure of the posts table can be seen below:

enter image description here

I am interested in evaluating the quality of the code I have written for the Create, Update and Delete operations. Also, In the post image management.

In the Post controller I have:

public function create() {

    // Only logged in users can create posts
    if (!$this->session->userdata('is_logged_in')) {
        redirect('login');
    }

    $data = $this->get_data();
    $data['tagline'] = "Add New Post";

    if ($data['categories']) {
        foreach ($data['categories'] as &$category) {
            $category->posts_count = $this->Posts_model->count_posts_in_category($category->id);
        }
    }

    $this->form_validation->set_rules('title', 'Title', 'required');
    $this->form_validation->set_rules('desc', 'Short description', 'required');
    $this->form_validation->set_rules('body', 'Body', 'required');
    $this->form_validation->set_error_delimiters('<p class="error-message">', '</p>');

    if($this->form_validation->run() === FALSE){
        $this->load->view('partials/header', $data);
        $this->load->view('dashboard/create-post');
        $this->load->view('partials/footer');
    } else {
        // Create slug (from title)
        $slug = url_title(convert_accented_characters($this->input->post('title')), 'dash', TRUE);
        $slugcount = $this->Posts_model->slug_count($slug, null);
        if ($slugcount > 0) {
            $slug = $slug."-".$slugcount;
        }

        // Upload image
        $config['upload_path'] = './assets/img/posts';
        $config['allowed_types'] = 'jpg|jpeg|png';
        $config['max_size'] = '2048';

        $this->load->library('upload', $config);

        if(!$this->upload->do_upload()){

            $errors = array('error' => $this->upload->display_errors());

            // Dysplay upload validation errors 
            // only if a file is uploaded and there are errors
            if (empty($_FILES['userfile']['name'])) {
                $errors = [];
            }

            if (empty($errors)) {
                $post_image = 'default.jpg';
            } else {
                $data['upload_errors'] = $errors;
            }

        } else {
            $data = array('upload_data' => $this->upload->data());
            $post_image = $_FILES['userfile']['name'];
        }

        if (empty($errors)) {
            $this->Posts_model->create_post($post_image, $slug);
            $this->session->set_flashdata('post_created', 'Your post has been created');
            redirect('/');
        } else {
            $this->load->view('partials/header', $data);
            $this->load->view('dashboard/create-post');
            $this->load->view('partials/footer');
        }
    }
}

public function edit($id) {
    // Only logged in users can edit posts
    if (!$this->session->userdata('is_logged_in')) {
        redirect('login');
    }

    $data = $this->get_data();
    $data['post'] = $this->Posts_model->get_post($id);

    if (($this->session->userdata('user_id') == $data['post']->author_id) || $this->session->userdata('user_is_admin')) {
        $data['tagline'] = 'Edit the post "' . $data['post']->title . '"';
        $this->load->view('partials/header', $data);
        $this->load->view('dashboard/edit-post');
        $this->load->view('partials/footer');
    } else {
        /* If the current user is not the author
        of the post do not alow edit */
        redirect('/' . $id);
    }
}

public function update() {
    // Form data validation rules
    $this->form_validation->set_rules('title', 'Title', 'required',  array('required' => 'The %s field can not be empty'));
    $this->form_validation->set_rules('desc', 'Short description', 'required',  array('required' => 'The %s field can not be empty'));
    $this->form_validation->set_rules('body', 'Body', 'required',  array('required' => 'The %s field can not be empty'));
    $this->form_validation->set_error_delimiters('<p class="error-message">', '</p>');

    $id = $this->input->post('id');

    // Update slug (from title)
    if ($this->form_validation->run()) {
        $slug = url_title(convert_accented_characters($this->input->post('title')), 'dash', TRUE);
        $slugcount = $this->Posts_model->slug_count($slug, $id);
        if ($slugcount > 0) {
            $slug = $slug."-".$slugcount;
        }
    } else {
        $slug = $this->input->post('slug');
    }

    // Upload image
    $config['upload_path'] = './assets/img/posts';
    $config['allowed_types'] = 'jpg|jpeg|png';
    $config['max_size'] = '2048';

    $this->load->library('upload', $config);

    if (isset($_FILES['userfile']['name']) && $_FILES['userfile']['name'] != null) {
        // Use name field in do_upload method
        if (!$this->upload->do_upload('userfile')) {

            $errors = array('error' => $this->upload->display_errors());

            // Display upload validation errors 
            // only if a file is uploaded and there are errors
            if (empty($_FILES['userfile']['name'])) {
                $errors = [];
            }

            if (!empty($errors)) {
                $data['upload_errors'] = $errors;
            }

        } else {
            $data = $this->upload->data();
            $post_image = $data['raw_name'].$data[ 'file_ext'];
        }
    }
    else {
        $post_image = $this->input->post('postimage');
    }

    if ($this->form_validation->run() && empty($errors)) {
        $this->Posts_model->update_post($id, $post_image, $slug);
        $this->session->set_flashdata('post_updated', 'Your post has been updated');
        redirect('/' . $slug);
    } else {
        $this->form_validation->run();
        $this->session->set_flashdata('errors', validation_errors());
        $this->session->set_flashdata('upload_errors', $errors);
        redirect('/dashboard/posts/edit/' . $slug);
    }
}

public function delete($slug) {
    // Only logged in users can delete posts
    if (!$this->session->userdata('is_logged_in')) {
        redirect('login');
    }

    $data['post'] = $this->Posts_model->get_post($slug);
    if (($this->session->userdata('user_id') == $data['post']->author_id) || $this->session->userdata('user_is_admin')) {
        $this->Posts_model->delete_post($slug);
        $this->session->set_flashdata('post_deleted', 'The post has been deleted');
        redirect('/');
    } else {
        /* If the current user is not the author
        of the post do not alow delete */
        $this->session->set_flashdata('no_permission_to_delete_post', 'You are not authorized to delete this post');
        redirect('/' . $slug);
    }
}

public function deleteimage($id) {
    $this->load->model('Posts_model');
    $this->Posts_model->delete_post_image($id);
    redirect($this->agent->referrer());
}

In the Posts_model model:

// Create, post
public function create_post($post_image, $slug) {
    $data = [
        'title' => $this->input->post('title'),
        'slug' => $slug,
        'description' => $this->input->post('desc'),
        'content' => $this->input->post('body'),
        'post_image' => $post_image,
        'author_id' => $this->session->userdata('user_id'),
        'cat_id' => $this->input->post('category'),
        'created_at' => date('Y-m-d H:i:s')
    ];
    return $this->db->insert('posts', $data);
}

// Update post
public function update_post($id, $post_image, $slug) {
    $data = [
        'title' => $this->input->post('title'),
        'slug' => $slug,
        'description' => $this->input->post('desc'),
        'content' => $this->input->post('body'),
        'post_image' => $post_image,
        'cat_id' => $this->input->post('category'),
        'updated_at' => date('Y-m-d H:i:s')
    ];

    $this->db->where('id', $id);
    return $this->db->update('posts', $data);
}

//Delete post
public function delete_post($slug) {
    $this->db->where('slug', $slug);
    $this->db->delete('posts');
    return true;
}

public function delete_post_image($id) {
    $this->db->update('posts', array('post_image'=>'default.jpg'), ['id'=>$id]);
}

The create-post.php view:

<div class="row">
    <?php $this->load->view("dashboard/partials/sidebar-single");?>
    <div class="col-sm-7 col-md-9">
      <div class="card bg-light">
        <h6 class="card-header text-dark">New Post</h6>
        <div class="card-body bg-white">
          <?php echo form_open_multipart(base_url('dashboard/posts/create')); ?>

          <div class="form-group <?php if(form_error('title')) echo 'has-error';?>">
            <input type="text" name="title" id="title" class="form-control" value="<?php echo set_value('title')?>" placeholder="Title">
            <?php if(form_error('title')) echo form_error('title'); ?> 
          </div>

          <div class="form-group <?php if(form_error('desc')) echo 'has-error';?>">
            <input type="text" name="desc" id="desc" class="form-control" value="<?php echo set_value('desc')?>" placeholder="Short decription">
            <?php if(form_error('desc')) echo form_error('desc'); ?> 
          </div>

          <div class="form-group <?php if(form_error('body')) echo 'has-error';?>">
            <textarea name="body" id="body" cols="30" rows="5" class="form-control" placeholder="Add post body">
              <?php echo set_value('body')?>
            </textarea>
            <?php if(form_error('body')) echo form_error('body'); ?> 
          </div>

          <div class="form-group">
            <select name="category" id="category" class="form-control">
              <?php foreach ($categories as $category): ?>
                <option value="<?php echo $category->id; ?>"><?php echo $category->name; ?></option>
              <?php endforeach; ?>
            </select>
          </div>

          <label for="postimage" id="imageUploader">Upload an image</label>
          <div class="form-group">
            <input type="file" name="userfile" id="postimage" size="20">
             <div class="error-messages">
              <?php if(isset($upload_errors)){
                foreach ($upload_errors as $upload_error) {
                  echo $upload_error;
                }
              }?>
            </div>
          </div>

          <div class="form-group d-flex">
            <div class="w-50 pr-1">
              <input type="submit" value="Save" class="btn btn-block btn-md btn-success">
            </div>
            <div class="w-50 pl-1">
              <a href="<?php echo base_url('dashboard'); ?>" class="btn btn-block btn-md btn-success">Cancel</a>
            </div>
          </div>

          <?php echo form_close(); ?>
        </div>
      </div>
    </div>
</div>

The edit-post.php view:

<div class="row">
    <?php $this->load->view("dashboard/partials/sidebar-single");?>
    <div class="col-sm-7 col-md-9">
      <div class="card bg-light">
        <h6 class="card-header text-dark">Edit post</h6>
        <div class="card-body bg-white">

          <?php if ($this->session->flashdata('errors')) {
           $errors = $this->session->flashdata('errors');
           echo '<div class="error-group alert alert-warning alert-dismissible">' . "\n";
           echo '<button type="button" class="close" data-dismiss="alert">&times;</button>' . "\n";
           echo $errors;
           echo '<p class="error-message">We have restored the post.</p>';
           echo '</div>';
         } ?>

         <?php echo form_open_multipart(base_url('dashboard/posts/update')); ?>
         <input type="hidden" name="id" id="pid" value="<?php echo $post->id; ?>">
         <input type="hidden" name="slug" id="slug" value="<?php echo $post->slug; ?>">

         <div class="form-group <?php if(form_error('title')) echo 'has-error';?>">
          <input type="text" name="title" id="title" class="form-control" placeholder="Title" value="<?php echo $post->title; ?>">
          <?php if(form_error('title')) echo form_error('title'); ?> 
        </div>

        <div class="form-group <?php if(form_error('desc')) echo 'has-error';?>">
          <input type="text" name="desc" id="desc" class="form-control" placeholder="Short decription" value="<?php echo $post->description; ?>">
          <?php if(form_error('desc')) echo form_error('desc'); ?> 
        </div>

        <div class="form-group <?php if(form_error('body')) echo 'has-error';?>">
          <textarea name="body" id="body" cols="30" rows="5" class="form-control" placeholder="Add post body"><?php echo $post->content; ?></textarea>
          <?php if(form_error('body')) echo form_error('body'); ?> 
        </div>

        <div class="form-group">
          <select name="category" id="category" class="form-control">
            <?php foreach ($categories as $category): ?>
              <?php if ($category->id == $post->cat_id): ?>
                <option value="<?php echo $category->id; ?>" selected><?php echo $category->name; ?></option>
                <?php else: ?>
                  <option value="<?php echo $category->id; ?>"><?php echo $category->name; ?></option>
                <?php endif; ?>
              <?php endforeach; ?>
            </select>
          </div>

          <input type="hidden" name="postimage" id="postimage" value="<?php echo $post->post_image; ?>">

          <label for="postimage" id="imageUploader">Upload an image</label>
          <div class="form-group">
            <input type="file" name="userfile" id="postimage" size="20"> 
            <?php
            if ($upload_errors = $this->session->flashdata('upload_errors')) {
              if ($this->session->flashdata('upload_errors')) { ?>              
                <div class="error-messages">
                  <?php if(isset($upload_errors)){
                    foreach ($upload_errors as $upload_error) {
                      echo $upload_error;
                    }
                  }?>
                </div>
              <?php }
            } ?>
          </div>

          <div class="form-group d-flex">
            <div class="w-50 pr-1">
              <input type="submit" value="Update" class="btn btn-block btn-md btn-success">
            </div>
            <div class="w-50 pl-1">
              <a href="<?php echo base_url('dashboard'); ?>" class="btn btn-block btn-md btn-success">Cancel</a>
            </div>
          </div>
          <?php echo form_close(); ?>
        </div>
      </div>
    </div>
</div>

The sidebar-single.php partial (that displays the post image):

<div class="card-list-group card bg-light mb-3">
    <h6 class="card-header text-dark">Featured Image</h6>
    <div class="card-body p-0 bg-white">
      <?php if (isset($post->post_image) && $post->post_image !== 'default.jpg'): ?>
        <img src="<?php echo base_url('assets/img/posts/') . $post->post_image; ?>" alt="Main Image of <?php echo $post->title; ?>" class="img-fluid">
      <?php else: ?>
        <img src="<?php echo base_url('assets/img/posts/') . 'default.jpg'; ?>" alt="Default Post Image" class="img-fluid">
      <?php endif ?>
    </div>
    <div class="card-footer p-2 bg-white text-center">
      <a href="#<?php echo isset($post->post_image) && $post->post_image !== 'default.jpg' ? '' : 'imageUploader' ?>" <?php echo isset($post->post_image) && $post->post_image !== 'default.jpg' ? 'data-pid="' . $post->id . '"' : '' ?> id="postImage" class="smooth-scroll">
        <?php echo isset($post->post_image) && $post->post_image !== 'default.jpg' ? 'Delete' : 'Add' ?> image
      </a>
    </div>
</div>

Posts deleting is dome via jQuery Ajax:

//Delete Posts
$('.delete-post').on('click', function(evt){
    evt.preventDefault();
    var deleteUrl = $(this).attr('href');
    var slug = $(this).data('slug');
    var postsCount = Number($("#posts_count").text());

    if(confirm('Delete this post?')) {
      if ($(this).hasClass("ajax-btn")) {
        $.ajax({
          url: baseUrl + '/dashboard/posts/delete/' + slug,
          method: 'GET',
          dataType: 'html',
          success: function(deleteMsg){
            postsCount = postsCount - 1;
            $('tr[data-slug="' + slug +'"]').fadeOut('250');
            $("#posts_count").text(postsCount);
            $('#post_delete_msg').text("The post has been deleted");
            $('#post_delete_msg').slideDown(250).delay(2500).slideUp(250);
          }
        });
      } else {
        window.location.href = deleteUrl;
      }
    }
});

The post image management (more exactly, deleteing the current post image) also makes use of jQuery Ajax:

$('#postImage').on('click', function(evt){
    evt.preventDefault();

    if (this.hash === "") {
      var $this = $(this);
      var $postImage = $this.closest('.card').find('img');
      var $hiddenPostImage = $('input[name="postimage"]');
      var defaultPostImage = baseUrl + 'assets/img/posts/default.jpg';

      //Get post ID
      var id = $(this).data('pid');

      if(confirm("Delete the post's featured image?")) {
        $.ajax({
          url: baseUrl + 'dashboard/posts/deleteimage/' + id,
          method: 'GET',
          dataType: 'html',
          success: function(deleteMsg){
            $postImage.attr('src', defaultPostImage);
            $hiddenPostImage.val(defaultPostImage);
            $this.text('Add image');
            $this.attr('href', '#imageUploader');
          }
        });
      }
    }
});

What could I have done better an how? :)

\$\endgroup\$
  • \$\begingroup\$ The title of the question must never be your concern about about your scripting, it is meant to uniquely express what your scripting does. Please edit. \$\endgroup\$ – mickmackusa Apr 13 at 1:45
  • \$\begingroup\$ I started to write a review, but then after I tried to refactor the if(!$this->upload->do_upload()){ condition block in the create() method of your Post controller, I fear that not all possibilities are accounted for and that your script may be working properly. \$\endgroup\$ – mickmackusa Apr 13 at 13:05
  • \$\begingroup\$ It looks like $tagline and the $category->posts_count property is never used. I think you should try to tighen up the logic a bit more before dumping so much code for review. \$\endgroup\$ – mickmackusa Apr 13 at 13:17
  • 2
    \$\begingroup\$ @mickmackusa This is already one of the provided close reasons. Under "Authorship of code" there's the section "that the code be embedded directly". Please don't needlessly use custom close reasons. Additionally there is enough code to write a review IMO, voting to close this just seems like you're reaching. \$\endgroup\$ – Peilonrayz Apr 15 at 0:10
  • 1
    \$\begingroup\$ @mickmackusa I agree, from your perspective closing seems correct and just. But I still think my perspective is correct too. \$\endgroup\$ – Peilonrayz Apr 15 at 0:32
2
\$\begingroup\$

Use Entities to represent you domain concerns and business rule.

Use repositories to comunicate with the database.

Upgrade to CodeIgniter 4

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.