Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ROAD 534] Add state to stories #309

Merged
merged 22 commits into from
Oct 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
16c3492
Add nokogiri for apple silicon
mateusdeap Sep 26, 2023
45149a9
Add `approved` field to the stories table
mateusdeap Sep 26, 2023
37225f9
Add `pending` method with specs
mateusdeap Sep 28, 2023
a0786cc
Add `update_story` controller action
mateusdeap Sep 28, 2023
974d647
Add frontend
mateusdeap Sep 28, 2023
3d79fda
Use path helper instead of url
mateusdeap Sep 28, 2023
fa11b34
Change button color with approval status
mateusdeap Sep 28, 2023
97e8dbf
Use enums for story status
mateusdeap Oct 2, 2023
87949c2
Use remote: true for status button and refactor routes
mateusdeap Oct 2, 2023
9528442
Add appropriate specs and modify existing ones to the new changes
mateusdeap Oct 2, 2023
342db07
Move story related css to appropriate file
mateusdeap Oct 2, 2023
740ea12
Change back to headless firefox
mateusdeap Oct 4, 2023
48cbbb2
Merge branch 'main' of github.com:fastruby/points into ROAD-534-add-a…
arielj Oct 9, 2023
e10e5ad
Refactor routes
mateusdeap Oct 9, 2023
08d344c
Apply suggestions from code review
mateusdeap Oct 9, 2023
a88dab2
Add status grid-area in scss
mateusdeap Oct 10, 2023
324c2e8
Refactor label update logic
mateusdeap Oct 10, 2023
85d21ed
Move status field to last position
mateusdeap Oct 10, 2023
c934650
Merge branch 'main' into ROAD-534-add-approved-attribute-to-stories
mateusdeap Oct 11, 2023
cf3af58
Remove Rejected as an option on story creation
mateusdeap Oct 11, 2023
a7bfb2c
Add specs to test status selection on story creation
mateusdeap Oct 11, 2023
38f4b44
Replace `row` for query selector
mateusdeap Oct 11, 2023
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
17 changes: 17 additions & 0 deletions app/assets/javascripts/stories.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,20 @@ document.addEventListener("DOMContentLoaded", () => {
});
});
});

function updateStatusButton(color, status) {
const button = document.querySelector(".story-title .dropdown-wrapper > button");
button.className = `button ${color}`;

const span = button.querySelector("span");
span.textContent = status;

document.querySelector(":focus").blur();
}

function updateStatusLabel(status, storyId) {
let row = document.getElementById(`story_${storyId}`)
status_label = row.querySelector(".status > .story-status-badge")
status_label.textContent = status
status_label.classList.value = `story-status-badge ${status}`
}
26 changes: 26 additions & 0 deletions app/assets/stylesheets/3-atoms/_badges.scss
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,29 @@
background-color: #57ce81;
}
}

.story-status-badge {
display: inline-block;
padding: 7px 15px;
font-weight: 600;
line-height: 1;
text-align: center;
white-space: nowrap;
vertical-align: baseline;
border-radius: 20px;
font-size: 11px;
background-color: $orange;

&.approved {
background-color: $green;
border-color: #1d4ed8;
color: $white;
}

&.rejected {
background-color: $magenta;
border-color: #d77e72;
color: $white;
}

}
2 changes: 1 addition & 1 deletion app/assets/stylesheets/4-molecules/_tables.scss
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
}
.project-table__row {
display: grid;
grid-template-columns: 1fr 70px 70px 260px;
grid-template-columns: 1fr 100px 70px 70px 260px;
align-items: center;
padding: 10px 0;
&.project-table__row--reports {
Expand Down
21 changes: 17 additions & 4 deletions app/assets/stylesheets/stories.scss
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,12 @@
word-break: break-all;
}

.story-description, .extra-info, .story_preview {
.story-description,
.extra-info,
.story_preview {
margin-bottom: 25px;
font-size: 15px;
p{
p {
margin-top: 1em;
}
em {
Expand Down Expand Up @@ -55,7 +57,6 @@
margin-bottom: 1.5em;
}


.modal p {
padding-bottom: 1.3em;
}
Expand All @@ -72,6 +73,7 @@
"title preview"
"description preview"
"extra extra-preview"
"status ."
"submit .";
grid-template-columns: repeat(2, minmax(50%, 1fr));
grid-column-gap: 10px;
Expand All @@ -94,6 +96,10 @@
&.story_extra_info {
grid-area: extra;
}

&.story_status {
grid-area: status;
}
}

.story_preview {
Expand All @@ -104,7 +110,8 @@
grid-area: extra-preview;
}

.extra_info_preview .content, .story_preview .content {
.extra_info_preview .content,
.story_preview .content {
overflow: auto;
max-height: min(50vh, 700px);
// prevent long links from overflowing
Expand All @@ -131,6 +138,12 @@
padding: 5px;
}

.story-title {
display: flex;
align-items: center;
gap: 1rem;
}

.comments-section {
margin: 16px 0;

Expand Down
27 changes: 24 additions & 3 deletions app/controllers/stories_controller.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
require "csv"
class StoriesController < ApplicationController
before_action :authenticate_user!
before_action :find_project, except: [:bulk_destroy, :render_markdown, :edit, :update, :destroy, :show, :move]
before_action :find_story, only: [:edit, :update, :destroy, :show, :move]
before_action :find_project, except: [:bulk_destroy, :render_markdown, :edit, :update, :destroy, :show, :move, :approve, :reject, :pending]
before_action :find_story, only: [:edit, :update, :destroy, :show, :move, :approve, :reject, :pending]
before_action :validate_url_product_id, only: [:edit, :update, :destroy, :show, :move]
before_action :ensure_unarchived!, except: [:show, :bulk_destroy, :render_markdown, :move]

Expand Down Expand Up @@ -127,6 +127,27 @@ def move
redirect_to @project
end

def approve
@story.approved!
respond_to do |format|
format.js { render "shared/update_status" }
end
end

def reject
@story.rejected!
respond_to do |format|
format.js { render "shared/update_status" }
end
end

def pending
@story.pending!
respond_to do |format|
format.js { render "shared/update_status" }
end
end

JuanVqz marked this conversation as resolved.
Show resolved Hide resolved
private

def find_project
Expand All @@ -143,7 +164,7 @@ def validate_url_product_id
end

def stories_params
params.require(:story).permit(:title, :description, :extra_info, :project_id)
params.require(:story).permit(:title, :description, :extra_info, :project_id, :status)
end

def expected_csv_headers?(file)
Expand Down
16 changes: 16 additions & 0 deletions app/helpers/stories_helper.rb
Original file line number Diff line number Diff line change
@@ -1,2 +1,18 @@
module StoriesHelper
def status_label(story)
arielj marked this conversation as resolved.
Show resolved Hide resolved
"<span class='story-status-badge #{story.status}'>#{story.status}</span>".html_safe
Copy link
Member

@JuanVqz JuanVqz Oct 11, 2023

Choose a reason for hiding this comment

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

I'd like to see the first letter as uppercase equal as we see it in the select field.
image

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I'm going to skip this one since I think labels are better in lowercase but, mainly, because when I started to change this, I noticed I'd have to edit quite a few specs and a bit of JS that was swapping css classes based on the text content of the label.

Maybe add this as a bug or improvement story for later on?

end

def status_color(story)
return "green" if @story.approved?
return "magenta" if @story.rejected?

"orange"
end

def options_for_status_select(story, action)
return options_for_select({"Pending" => "pending", "Approved" => "approved"}, selected: story.status) if action == "new"

options_for_select({"Pending" => "pending", "Approved" => "approved", "Rejected" => "rejected"}, selected: story.status)
end
end
2 changes: 2 additions & 0 deletions app/models/story.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ class Story < ApplicationRecord

before_create :add_position

enum :status, [:pending, :approved, :rejected]

scope :by_position, -> { order("stories.position ASC NULLS FIRST, stories.created_at ASC") }

def best_estimate_average
Expand Down
12 changes: 6 additions & 6 deletions app/views/estimates/_update_row.js.erb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
let row = document.getElementById("story_<%= estimate.story_id %>")
row.children[1].innerText = "<%= j(estimate.best_case_points.to_s) %>"
row.children[2].innerText = "<%= j(estimate.worst_case_points.to_s) %>"
updateStatusLabel("<%= estimate.story.status %>", "<%= estimate.story_id %>")

let totals_row = document.querySelector('.project-table tfoot tr')
totals_row.children[1].innerText = "<%= j @project.best_estimate_sum_per_user(current_user) %>"
totals_row.children[2].innerText = "<%= j @project.worst_estimate_sum_per_user(current_user) %>"
document.getElementById("best_estimate_<%= estimate.story_id %>").innerText = "<%= j(estimate.best_case_points.to_s) %>"
document.getElementById("worst_estimate_<%= estimate.story_id %>").innerText = "<%= j(estimate.worst_case_points.to_s) %>"

document.querySelector('.project-table tfoot tr > .best_estimates_total').innerText = "<%= j @project.best_estimate_sum_per_user(current_user) %>"
document.querySelector('.project-table tfoot tr > .worst_estimates_total').innerText = "<%= j @project.worst_estimate_sum_per_user(current_user) %>"
4 changes: 2 additions & 2 deletions app/views/estimates/create.js.erb
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
(function(){
<% if @estimate.persisted? %>
<%= render partial: 'update_row', locals: {estimate: @estimate} %>
const addEstimate = row.querySelector('.add-estimate')
const addEstimate = document.getElementById("story_<%= @estimate.story_id %>").querySelector('.add-estimate')
addEstimate.insertAdjacentHTML('afterend', "<%= j(link_to 'Edit Estimate', edit_project_story_estimate_path(@project.id, @estimate.story, @estimate.id), class: "button edit-estimate", remote: true) %>")
addEstimate.remove()
closeModal()
<% else %>
updateModal("New estimate", "<%= j(render partial: 'modal_body') %>")
<% end %>
})()
})()
13 changes: 8 additions & 5 deletions app/views/projects/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
<thead class="table-header fixed-header">
<tr class="project-table__row project-table__row--header">
<th class="project-table__cell">Story Title</th>
<th class="project-table__cell">Status</th>
arielj marked this conversation as resolved.
Show resolved Hide resolved
<th class="project-table__cell">Best<br />Estimate</th>
<th class="project-table__cell">Worst<br />Estimate</th>
<th class="project-table__cell story_actions">
Expand All @@ -25,14 +26,15 @@
<tbody id="stories" data-url="<%= sort_stories_project_path(@project.id) %>" data-unlocked="<%= is_unlocked?(@project) %>">
<% if @stories.present? %>
<% @stories.each do | story | %>
<tr class="project-table__row project-table__row--story" id="<%= dom_id(story)%>" >
<tr class="project-table__row project-table__row--story" id="<%= dom_id(story)%>" data-status="<%= story.status %>">
<td class="project-table__cell">
<span class="popup">Copied to clipboard</span>
<input type="checkbox" name="stories[]" value="<%= story.id %>">
<%= link_to "#{story.id} - #{story.title}", [story.project, story] %>
</td>
<td class="project-table__cell"><%= story.estimate_for(current_user)&.best_case_points %></td>
<td class="project-table__cell"><%= story.estimate_for(current_user)&.worst_case_points %></td>
<td class="project-table__cell status"><%= status_label(story) %></td>
<td class="project-table__cell" id="best_estimate_<%= story.id %>"><%= story.estimate_for(current_user)&.best_case_points %></td>
<td class="project-table__cell" id="worst_estimate_<%= story.id %>"><%= story.estimate_for(current_user)&.worst_case_points %></td>
<td class="project-table__cell story_actions">
<% if is_unlocked?(@project) %>
<% if estimated(story) %>
Expand Down Expand Up @@ -81,8 +83,9 @@
<tfoot>
<tr class="project-table__row project-table__row--footer">
<td class="project-table__cell">Total estimates</td>
<td class="project-table__cell"><%= @project.best_estimate_sum_per_user(current_user) %></td>
<td class="project-table__cell"><%= @project.worst_estimate_sum_per_user(current_user) %></td>
<td class="project-table__cell"></td>
<td class="project-table__cell best_estimates_total"><%= @project.best_estimate_sum_per_user(current_user) %></td>
<td class="project-table__cell worst_estimates_total"><%= @project.worst_estimate_sum_per_user(current_user) %></td>
<td class="project-table__cell"></td>
</tr>
</tfoot>
Expand Down
15 changes: 14 additions & 1 deletion app/views/shared/_story.html.erb
Original file line number Diff line number Diff line change
@@ -1,4 +1,17 @@
<h4 class="word-break">Story #<%= story.id %>: <%= story.title %></h4>
<div class="story-title">
<h4 class="word-break">Story #<%= story.id %>: <%= story.title %></h4>
<div class="dropdown-wrapper">
<button class="button <%= status_color(story) %>">
<span><%= story.status %></span>
<i class="fa fa-solid fa-caret-down"></i>
</button>
<div class="dropdown">
<%= button_to "Approve", approve_story_path(@story), class: "status-selector", method: :patch, remote: true %>
<%= button_to "Reject", reject_story_path(@story), class: "status-selector", method: :patch, remote: true %>
<%= button_to "Pending", pending_story_path(@story), class: "status-selector", method: :patch, remote: true %>
</div>
</div>
</div>

<div class="story-description">
<%= markdown(story.description) %>
Expand Down
2 changes: 2 additions & 0 deletions app/views/shared/update_status.js.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
updateStatusButton("<%= status_color(@story) %>", "<%= @story.status %>");
updateStatusLabel("<%= @story.status %>", "<%= @story.id %>")
5 changes: 5 additions & 0 deletions app/views/stories/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@
<div class="content"><%= markdown(@story.extra_info) %></div>
</div>

<div class="field story_status">
<%= f.label :status, "Status" %>
<%= f.select :status, options_for_status_select(@story, action_name), class: "project-story-approved" %>
</div>


<div class="btn-group">
<%= f.submit yield(:button_text), class: "button green", id: "edit" %>
Expand Down
14 changes: 11 additions & 3 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,17 @@
get "home/index"
get "reports/index"

resource :stories do
post :bulk_destroy, to: "stories#bulk_destroy"
post :render_markdown
resources :stories do
member do
patch :approve
patch :reject
patch :pending
end

collection do
post :bulk_destroy, to: "stories#bulk_destroy"
post :render_markdown
end
end

resources :projects do
Expand Down
5 changes: 5 additions & 0 deletions db/migrate/20230829001347_add_status_to_stories.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddStatusToStories < ActiveRecord::Migration[7.0]
def change
add_column :stories, :status, :integer, default: 0
end
end
1 change: 1 addition & 0 deletions db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
t.integer "position"
t.integer "real_score"
t.string "extra_info"
t.integer "status", default: 0
end

create_table "users", force: :cascade do |t|
Expand Down
27 changes: 27 additions & 0 deletions spec/controllers/stories_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,33 @@
end
end

describe "#approve" do
it "updates the story status to approved" do
patch :approve, params: {id: story.id}, format: :js

expect(story.reload.status).to eq("approved")
expect(response).to render_template("shared/update_status")
end
end

describe "#reject" do
it "updates the story status to rejected" do
patch :reject, params: {id: story.id}, format: :js

expect(story.reload.status).to eq("rejected")
expect(response).to render_template("shared/update_status")
end
end

describe "#pending" do
it "updates the story status to pending" do
patch :pending, params: {id: story.id}, format: :js

expect(story.reload.status).to eq("pending")
expect(response).to render_template("shared/update_status")
end
end

describe "#export" do
it "exports a CSV file" do
get :export, params: {project_id: project.id}
Expand Down
Loading
Loading