Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 20 additions & 10 deletions engine/app/controllers/coplan/plans_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,35 @@ module CoPlan
class PlansController < ApplicationController
before_action :set_plan, only: [:show, :edit, :update, :update_status, :toggle_checkbox, :history]

PER_PAGE = 20

def index
@plans = Plan.includes(:plan_type, :tags)
plans = Plan.includes(:plan_type, :tags, :created_by_user)
.where.not(status: "brainstorm")
.or(Plan.where(created_by_user: current_user))
.order(updated_at: :desc)
@plans = @plans.where(status: params[:status]) if params[:status].present?
@plans = @plans.where(created_by_user: current_user) if params[:scope] == "mine"
@plans = @plans.where(plan_type_id: params[:plan_type]) if params[:plan_type].present?
@plans = @plans.with_tag(params[:tag]) if params[:tag].present?
.order(updated_at: :desc, id: :desc)
plans = plans.where(status: params[:status]) if params[:status].present?
plans = plans.where(created_by_user: current_user) if params[:scope] == "mine"
plans = plans.where(plan_type_id: params[:plan_type]) if params[:plan_type].present?
plans = plans.with_tag(params[:tag]) if params[:tag].present?

@plan_types = PlanType.order(:name)
@page = (params[:page] || 1).to_i
@plans = plans.limit(PER_PAGE + 1).offset((@page - 1) * PER_PAGE)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Stabilize sort key before offset pagination

Using offset pagination here without a deterministic tie-breaker means pages can skip or duplicate plans when multiple records share the same updated_at value. The query still orders only by updated_at, so frame page=2 can return overlapping or missing rows relative to page=1 for ties, which breaks infinite scroll correctness; add a secondary unique order (for example id) before applying offset.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — added id: :desc as a secondary sort key in 283dc0d to make the offset pagination deterministic.

@has_next_page = @plans.size > PER_PAGE
@plans = @plans.first(PER_PAGE)

@plan_unread_counts = current_user.notifications.unread
.where(plan_id: @plans.select(:id))
.where(plan_id: @plans.map(&:id))
.group(:plan_id)
.count

@show_onboarding_banner = CoPlan.configuration.onboarding_banner.present? &&
!current_user.created_plans.exists?
if turbo_frame_request?
render partial: "coplan/plans/plan_page", locals: { plans: @plans, plan_unread_counts: @plan_unread_counts, page: @page, has_next_page: @has_next_page }, layout: false
else
@plan_types = PlanType.order(:name)
@show_onboarding_banner = CoPlan.configuration.onboarding_banner.present? &&
!current_user.created_plans.exists?
end
end

def show
Expand Down
33 changes: 33 additions & 0 deletions engine/app/views/coplan/plans/_plan_page.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<%= turbo_frame_tag "plans-page-#{page}" do %>
<% plans.each do |plan| %>
<div class="card plans-list__item">
<div class="plans-list__header">
<%= link_to plan.title, plan_path(plan), class: "plans-list__title", data: { turbo_frame: "_top" } %>
<span class="badge badge--<%= plan.status %>"><%= plan.status %></span>
<% if plan.plan_type %>
<span class="badge badge--type"><%= plan.plan_type.name %></span>
<% end %>
<% plan_unread = plan_unread_counts[plan.id] || 0 %>
<% if plan_unread > 0 %>
<span class="inbox-badge"><%= plan_unread %></span>
<% end %>
</div>
<% if plan.tags.any? %>
<div class="plans-list__tags">
<% plan.tags.each do |tag| %>
<%= link_to tag.name, plans_path(params.permit(:scope, :status, :plan_type).merge(tag: tag.name)), class: "badge badge--tag #{'badge--tag-active' if params[:tag] == tag.name}", data: { turbo_frame: "_top" } %>
<% end %>
</div>
<% end %>
<div class="plans-list__meta text-sm text-muted">
<%= user_avatar(plan.created_by_user) %> <%= plan.created_by_user.name %> · v<%= plan.current_revision %> · updated <%= time_ago_in_words(plan.updated_at) %> ago
</div>
</div>
<% end %>

<% if has_next_page %>
<%= turbo_frame_tag "plans-page-#{page + 1}", src: plans_path(params.permit(:scope, :status, :plan_type, :tag).merge(page: page + 1)), loading: :lazy do %>
<div class="plans-list__loading text-muted text-sm">Loading more plans…</div>
<% end %>
<% end %>
<% end %>
26 changes: 1 addition & 25 deletions engine/app/views/coplan/plans/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -32,31 +32,7 @@

<% if @plans.any? %>
<div class="plans-list">
<% @plans.each do |plan| %>
<div class="card plans-list__item">
<div class="plans-list__header">
<%= link_to plan.title, plan_path(plan), class: "plans-list__title" %>
<span class="badge badge--<%= plan.status %>"><%= plan.status %></span>
<% if plan.plan_type %>
<span class="badge badge--type"><%= plan.plan_type.name %></span>
<% end %>
<% plan_unread = @plan_unread_counts[plan.id] || 0 %>
<% if plan_unread > 0 %>
<span class="inbox-badge"><%= plan_unread %></span>
<% end %>
</div>
<% if plan.tags.any? %>
<div class="plans-list__tags">
<% plan.tags.each do |tag| %>
<%= link_to tag.name, plans_path(params.permit(:scope, :status, :plan_type).merge(tag: tag.name)), class: "badge badge--tag #{'badge--tag-active' if params[:tag] == tag.name}" %>
<% end %>
</div>
<% end %>
<div class="plans-list__meta text-sm text-muted">
<%= user_avatar(plan.created_by_user) %> <%= plan.created_by_user.name %> · v<%= plan.current_revision %> · updated <%= time_ago_in_words(plan.updated_at) %> ago
</div>
</div>
<% end %>
<%= render partial: "coplan/plans/plan_page", locals: { plans: @plans, plan_unread_counts: @plan_unread_counts, page: @page, has_next_page: @has_next_page } %>
</div>
<% else %>
<div class="empty-state card">
Expand Down
Loading