refactor: clean up backend code

app/controllers/projects/tasks_controller.rb

@@ -1,8 +1,10 @@
 # frozen_string_literal: true
 
 class Projects::TasksController < ApplicationController
-  before_action :set_task, only: %i[edit update set_percentage assign_self destroy]
-  before_action :set_project, except: :set_percentage
+  before_action :set_task,
+                only: %i[edit update set_percentage assign_self destroy]
+  before_action :set_project,
+                except: %i[update destroy assign_self set_percentage]
   before_action :set_skills, only: %i[new edit]
 
   layout 'project'
@@ -53,7 +55,7 @@ def data
     authorize! @project, to: :count?
     return render_404 unless request.xhr? && valid_data_type?
 
-    scope_data = authorized_scope(Task.where(project: @project), as: @type)
+    scope_data = authorized_scope(@project.tasks, as: @type)
 
     user_data = scope_data.user_data
     images = assignee_avatars(user_data.pluck('users.id').uniq)
@@ -63,11 +65,7 @@ def data
   end
 
   def set_percentage
-    if @task.update(edit_params)
-      head :ok
-    else
-      task_fail
-    end
+    @task.update(edit_params) ? head(:ok) : task_fail
   end
 
   private
@@ -77,17 +75,16 @@ def set_project
   end
 
   def set_skills
-    @skills = Skill.where(category: @project.category)
-                   .collect { |s| [s.name, s.id] }
+    @skills = @project.category.skills.map { |s| [s.name, s.id] }
 
     team = current_user.teams.find_by(project: @project)
     @assignees = authorized_scope(
       @project.users,
-      as: :assignee, scope_options: { team_id: team&.id, project: @project }
+      as: :assignee,
+      scope_options: { team_id: team&.id, project: @project }
     )
   end
 
-  # Use callbacks to share common setup or constraints between actions.
   def set_task
     @task = Task.find(params[:id])
     authorize! @task

app/controllers/projects/teams_controller.rb

@@ -2,22 +2,17 @@
 
 class Projects::TeamsController < ApplicationController
   before_action :set_team, only: %i[edit show update destroy remove_user]
-  before_action :set_project, except: %i[show update destroy remove_user]
+  before_action :set_project, except: %i[show update remove_user]
   before_action :set_skills, only: %i[new edit]
 
   layout 'project'
 
-  # GET /teams
-  def manage; end
-
   def manage_data
     return render_404 unless request.xhr?
 
-    @data = User.joins(:teams).where(teams: { project: @project })
-                .select('users.*, teams.id as team_id')
     render json: {
-      data: @data.group_by(&:team_id),
-      teams: Team.where(project: @project).select(:id, :name, :team_size)
+      data: @project.users.select('users.*, teams.id AS team_id').group_by(&:team_id),
+      teams: @project.teams.select(:id, :name, :team_size)
     }
   end
 
@@ -74,8 +69,7 @@ def update
 
   # DELETE /teams/1
   def destroy
-    team = Team.find_by(project_id: params[:project_id], name: 'Unassigned')
-    team.users << @team.users
+    @project.unassigned_team.users << @team.users
     @team.destroy
     head :ok
   end
@@ -91,15 +85,13 @@ def remove_user
 
   private
 
-  # Use callbacks to share common setup or constraints between actions.
   def set_team
     @team = Team.find(params[:id])
     authorize! @team
   end
 
   def set_skills
-    @skills = Skill.where(category: @project.category)
-                   .collect { |s| [s.name, s.id] }
+    @skills = @project.category.skills.map { |s| [s.name, s.id] }
   end
 
   def set_project

app/controllers/projects_controller.rb

@@ -45,24 +45,28 @@ def update
   end
 
   def change_status
-    msg = prepare_message
-    if @project.update(status: params[:status])
-      project_success(msg)
-    else
-      project_fail
-    end
+    msg = case params[:status]
+          when 'completed'
+            'Project Archived'
+          when 'active'
+            @project.appeals.application.destroy_all
+            @project.completed? ? 'Project Activated' : 'Project Closed'
+          when 'open'
+            'Project Opened'
+          end
+
+    @project.update(status: params[:status]) ? project_success(msg) : project_fail
   end
 
   def count
-    return head :bad_request unless
-    params.key?(:type) && %w[task application].include?(params[:type])
-
-    count = case params[:type]
-            when 'task' then @project.tasks.where('percentage < ?', 100).size
-            when 'application' then @project.appeals.size
-            end
-
-    render json: { count: count }
+    return head :bad_request unless %w[task application].include?(params[:type])
+
+    render json: {
+      count: case params[:type]
+             when 'task' then @project.tasks.where('percentage < 100').size
+             when 'application' then @project.appeals.size
+             end
+    }
   end
 
   private
@@ -100,16 +104,4 @@ def project_success(message)
   def project_fail(message = @project.errors.full_messages)
     render json: { message: message }, status: :unprocessable_entity
   end
-
-  def prepare_message
-    case params[:status]
-    when 'completed'
-      'Project Archived'
-    when 'active'
-      @project.appeals.where(type: 'application').destroy_all
-      @project.completed? ? 'Project Activated' : 'Project Closed'
-    when 'open'
-      'Project Opened'
-    end
-  end
 end

app/controllers/users_controller.rb

@@ -18,7 +18,6 @@ def timeline
     return render_404 unless request.xhr?
 
     month = find_next_activity(params[:month].to_i)
-
     month.present? ? render_timeline(month) : head(:no_content)
   end
 
@@ -29,17 +28,19 @@ def set_user
   end
 
   def find_next_activity(month)
-    data = Activity.where('user_id = ? AND created_at <= ?',
-                          @user.id,
-                          DateTime.current.beginning_of_month << month)
+    data = @user.activities.where('created_at <= ?',
+                                  DateTime.current.beginning_of_month << month)
 
     data.order('created_at DESC').first.created_at.to_datetime if data.exists?
   end
 
   def render_timeline(mth)
     tasks, events = authorized_scope(@user.activities.from_month(mth))
-    html = view_to_html_string('users/_timeline', events: events, tasks: tasks,
-                                                  header: mth.strftime('%B %Y'))
+    html = view_to_html_string('users/_timeline',
+                               events: events,
+                               tasks: tasks,
+                               header: mth.strftime('%B %Y'))
+
     render json: { html: html, m: ((Time.current - mth) / 1.month).floor }
   end
 

app/models/activity.rb

@@ -27,8 +27,5 @@ class Activity < ApplicationRecord
 
   validates :key, presence: true
 
-  def self.from_month(mth)
-    where('activities.created_at >= ? AND activities.created_at <= ?',
-          mth.beginning_of_month, mth.end_of_month)
-  end
+  scope :from_month, ->(mth) { where(created_at: mth.all_month) }
 end

app/models/project.rb

@@ -110,8 +110,6 @@ def create_activity
   end
 
   def complete_activity
-    return unless completed?
-
-    activities.find_or_create_by(key: 'project/complete', user: user)
+    activities.find_or_create_by(key: 'project/complete', user: user) if completed?
   end
 end

app/models/task.rb

@@ -50,7 +50,7 @@ class Task < ApplicationRecord
 
   scope :user_data, lambda {
     joins(:users)
-      .select('tasks.id, users.id as user_id, users.name as user_name')
+      .select('tasks.id, users.id AS user_id, users.name AS user_name')
       .group('tasks.id, users.id')
   }
 
@@ -63,10 +63,6 @@ class Task < ApplicationRecord
                 .group('tasks.id, users.name')
   }
 
-  def user_ids
-    task_users.pluck(:user_id)
-  end
-
   def completed?
     percentage == 100
   end

app/models/team.rb

@@ -44,9 +44,7 @@ class Team < ApplicationRecord
   private
 
   def check_users_limit(_)
-    project.check_users_limit
-
-    return unless users.size > team_size
+    return project.check_users_limit unless users.size > team_size
 
     errors[:base] << 'Team Size is smaller than total members'
   end

app/policies/project_policy.rb

@@ -36,7 +36,7 @@ class ProjectPolicy < ApplicationPolicy
   end
 
   def show?
-    record.visibility || owner? || user&.admin? || user&.in_project?(record) ||
+    record.visibility || manage? || user&.in_project?(record) ||
       record.appeals.invitation.where(user: user).exists?
   end
 
@@ -49,7 +49,7 @@ def count?
   end
 
   def change_visibility?
-    user&.admin? || !user.license.free?
+    user&.admin? || !user.license.free? && (owner? || record.new_record?)
   end
 
   def create_task?

app/views/projects/_settings.haml

@@ -53,6 +53,6 @@
     = button_to 'Reactivate Project', change_status_project_path(@project), method: :patch, remote: true, params: { status: 'active' }, data: { confirm: 'Are you sure?' }, class: 'button is-warning', form: { '@ajax:error': 'ajaxError' }
   - else
     %h1.title.is-2
-      Mark Project as Complete
+      Mark Project as Completed
     %p.has-text-grey Marking the project as complete will not allow anyone to change the project
     = button_to 'Archive Project', change_status_project_path(@project), method: :patch, remote: true, params: { status: 'completed' }, data: { confirm: 'Are you sure?' }, class: 'button is-danger', form: { '@ajax:error': 'ajaxError' }

app/views/projects/new.haml

@@ -44,7 +44,7 @@
             %p.text-sm.mt-2
               Be concise and informative, only the first 117 characters will be shown in the project list.
 
-        - if allowed_to? :change_visibility?, Project
+        - if allowed_to? :change_visibility?, Project.new
           .field.mb-4
             .control
               %label.label.required Visibility Level

app/views/projects/tasks/edit.haml

@@ -5,7 +5,7 @@
     %h1.title.is-4 Edit Task
     .box
       = form_with model: @task, url: [@project, @task], html: { '@ajax:error': 'ajaxError' } do |f|
-        .field.my-4
+        .field
           %label.label.required Task Name
           .control
             = f.text_field :name, class: 'input', required: ''

app/views/projects/tasks/new.haml

@@ -6,7 +6,7 @@
     %p.subtitle.is-6 Create Task to delegate work to your fellow members.
     .box
       = form_with model: @task, url: [@project, @task], html: { '@ajax:error': 'ajaxError' } do |f|
-        .field.my-4
+        .field
           %label.label.required Task Name
           .control
             = f.text_field :name, class: 'input', required: ''

app/views/projects/teams/edit.haml

@@ -19,5 +19,5 @@
             = f.select :skill_ids, @skills, {}, multiple: true
         .field.mt-8
           .control.flex.justify-between
-            = f.submit 'Save Team', class: 'button is-success'
+            = f.submit 'Save', class: 'button is-success'
             = link_to 'Cancel', manage_project_teams_path(@team.project.id), class: 'button'

app/views/projects/teams/manage.haml

@@ -7,7 +7,7 @@
         %h1.title.is-2
         Manage Teams
       .column
-        - if user_signed_in? && (current_user == @project.user || current_user.admin?) && [email protected]?
+        - if (current_user == @project.user || current_user.admin?) && [email protected]?
           .float-right.buttons
             = link_to new_project_team_path(project_id: @project.id), class: 'button is-primary' do
               %p New Team

config/routes.rb

@@ -148,7 +148,7 @@
         end
       end
 
-      resources :tasks do
+      resources :tasks, except: %i[index show] do
         collection do
           get 'data'
         end

spec/factories/newsletter_feedbacks.rb

@@ -22,14 +22,10 @@
 
 FactoryBot.define do
   factory :newsletter_feedback, class: NewsletterFeedback.name do
-    subscribed
-
-    trait :subscribed do
-      newsletter_subscription
-    end
+    association :newsletter_subscription, factory: :newsletter_subscription
 
     trait :not_subscribed do
-      association :newsletter_subscription, :unsubscribed
+      association :newsletter_subscription, :unsubscribed, factory: :newsletter_subscription
     end
 
     trait :no_longer do

spec/factories/teams.rb

@@ -26,9 +26,5 @@
     name { generate(:name) }
     team_size { 5 }
     association :project, factory: :project
-
-    trait :with_skill do
-      association :skills, factory: :skill
-    end
   end
 end

spec/models/project_spec.rb

@@ -64,10 +64,8 @@
   end
 
   describe 'before_validation hook' do
-    describe '#check_users_limit' do
-      let(:project) do
-        create(:project_with_members, user: create(:user), members_count: 9)
-      end
+    describe '#validate_status_update' do
+      let(:project) { create(:project_with_members, user: create(:user), members_count: 9) }
 
       before do
         project.reload # otherwise project.users won't be updated