maverick/docbrown
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix up articles controller (#2855)

Browse source 
* Fix up articles controller

* Update request specs

* Add proper post_under_org tests

* Remove create_job_opportunity

* Add user profile back in
This commit is contained in:
Ben Halpern 2019-05-16 13:56:26 -04:00 • committed by GitHub
parent 68f06c2076
commit fb6904f86d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 69 additions and 135 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
app/assets/stylesheets/preact/article-form.scss
View file

@ -374,7 +374,7 @@
display: inline-block;
padding: 2px 15px;
border-radius: 3px;
width: 100px;
width: 110px;
@media screen and (min-width: 600px) {
margin-left: -5px;
display: none;

97
app/controllers/articles_controller.rb
View file

@ -121,64 +121,31 @@ class ArticlesController < ApplicationController
@user = current_user
respond_to do |format|
format.html do
@article = ArticleCreationService.
new(@user, article_params, job_opportunity_params: job_opportunity_params).
create!
@article = ArticleCreationService.new(@user, article_params_json).create!
redirect_after_creation
end
format.json do
@article = ArticleCreationService.new(@user, article_params_json).create!
render json: if @article.persisted?
@article.to_json(only: [:id], methods: [:current_state_path])
else
@article.errors.to_json
end
end
end
render json: if @article.persisted?
@article.to_json(only: [:id], methods: [:current_state_path])
else
@article.errors.to_json
end
end
def update
authorize @article
@user = @article.user || current_user
not_found if @article.user_id != @user.id && !@user.has_role?(:super_admin)
edited_at_date = if @article.user == current_user && @article.published
Time.current
else
@article.edited_at
end
if request.format.json?
not_found if @article.user_id != @user.id && !@user.has_role?(:super_admin)
render json: if @article.update(article_params_json)
@article.to_json(only: [:id], methods: [:current_state_path])
else
@article.errors.to_json
end
else
@article.tag_list = []
@article.main_image = nil
edited_at_date = if @article.user == current_user && @article.published
Time.current
else
@article.edited_at
end
if @article.update(article_params.merge(edited_at: edited_at_date))
handle_org_assignment
handle_hiring_tag
if @article.published
# why does it send a notification each time the published flag is true?
Notification.send_to_followers(@article, "Published") if @article.saved_changes["published_at"]&.include?(nil)
path = @article.path
else
Notification.remove_all_without_delay(notifiable_id: @article.id, notifiable_type: "Article", action: "Published")
path = "/#{@article.username}/#{@article.slug}?preview=#{@article.password}"
end
redirect_to(params[:destination] || path)
else
redirect_to :edit
end
end
render json: if @article.update(article_params_json.merge(edited_at: edited_at_date))
Notification.send_to_followers(@article, "Published") if @article.published && @article.saved_changes["published_at"]&.include?(nil)
@article.to_json(only: [:id], methods: [:current_state_path])
else
@article.errors.to_json
end
end
def delete_confirm
@ -209,14 +176,6 @@ class ArticlesController < ApplicationController
end
end
def handle_hiring_tag
if job_opportunity_params.present? && @article.tag_list.include?("hiring")
create_or_update_job_opportunity
elsif @article.job_opportunity && !@article.tag_list.include?("hiring")
@article.job_opportunity.destroy!
end
end
def handle_user_or_organization_feed
if (@user = User.find_by(username: params[:username]))
@articles = @articles.where(user_id: @user.id)
@ -234,16 +193,6 @@ class ArticlesController < ApplicationController
@articles = @articles.cached_tagged_with(@tag)
end
def create_or_update_job_opportunity
if @article.job_opportunity.present?
@article.job_opportunity.update(job_opportunity_params)
else
@job_opportunity = JobOpportunity.create(job_opportunity_params)
@article.job_opportunity = @job_opportunity
@article.save
end
end
def set_article
owner = User.find_by(username: params[:username]) || Organization.find_by(slug: params[:username])
found_article = if params[:slug]
@ -270,6 +219,7 @@ class ArticlesController < ApplicationController
elsif params["article"]["series"] == ""
params["article"]["collection_id"] = nil
end
if params["article"]["version"] == "v1"
params.require(:article).permit(
:body_markdown, :organization_id
@ -282,15 +232,6 @@ class ArticlesController < ApplicationController
end
end
def job_opportunity_params
return nil if params[:article][:job_opportunity].blank?
params[:article].require(:job_opportunity).permit(
:remoteness, :location_given, :location_city, :location_postal_code,
:location_country_code, :location_lat, :location_long
)
end
def redirect_after_creation
@article.decorate
if @article.persisted?

18
app/javascript/article-form/articleForm.jsx
View file

@ -139,11 +139,19 @@ export default class ArticleForm extends Component {
};
showPreview = response => {
this.setState({
previewShowing: true,
helpShowing: false,
previewResponse: response,
});
if (response.processed_html) {
this.setState({
previewShowing: true,
helpShowing: false,
previewResponse: response,
errors: null,
});
} else {
this.setState({
errors: response,
submitting: false,
});
}
};
toggleOrgPosting = e => {

15
app/services/article_creation_service.rb
View file

@ -1,8 +1,7 @@
class ArticleCreationService
def initialize(user, article_params, job_opportunity_params: {})
def initialize(user, article_params)
@user = user
@article_params = article_params
@job_opportunity_params = job_opportunity_params
end
def create!
@ -27,24 +26,14 @@ class ArticleCreationService
article.organization = user.organization if publish_under_org
article.collection = Collection.find_series(series, user) if series.present?
create_job_opportunity(article)
if article.save!
Notification.send_to_followers(article, "Published") if article.published
end
article.decorate
end
def create_job_opportunity(article)
return if job_opportunity_params.blank?
job_opportunity = JobOpportunity.create(job_opportunity_params)
article.job_opportunity = job_opportunity
raise unless article.tag_list.include? "hiring"
end
private
attr_reader :user, :job_opportunity_params
attr_reader :user
attr_accessor :article_params
end

10
app/views/articles/_v2_form.html.erb
View file

@ -3,8 +3,8 @@
id="article-form"
data-article="<%= @article.to_json(only: %i[id title cached_tag_list published body_markdown main_image organization_id canonical_url], methods: %i[series all_series]) %>"
data-organization="<%= @organization&.to_json(only: %i[name bg_color_hex text_color_hex], methods: [:profile_image_90]) %>"
data-version="<%= current_user.editor_version%>">
<form class="articleform__form articleform__form--<%= current_user.editor_version %>">
data-version="<%= @user.editor_version%>">
<form class="articleform__form articleform__form--<%= @user.editor_version %>">
<% if @organization %>
<div class="articleform__orgsettings">
<img src="<%= @organization.profile_image_90 %>" style="opacity: <%= @article.organization ? "1.0" : "0.7" %>" alt="<%= @organization.name %> profile image">
@ -14,7 +14,7 @@
</button>
</div>
<% end %>
<% if current_user.editor_version == "v2" %>
<% if @user.editor_version == "v2" %>
<% if @article.main_image.present? %>
<div class="articleform__mainimage"><img src="<%= @article.main_image %>"></div>
<% end %>
@ -30,7 +30,7 @@
<textarea class="articleform__body" placeholder="Body Markdown" name="body_markdown"><%= @article.body_markdown %></textarea>
<div>
<button class="articleform__detailsButton articleform__detailsButton--image articleform__detailsButton--bottom"></button>
<% if current_user.editor_version == "v2" %>
<% if @user.editor_version == "v2" %>
<button class="articleform__detailsButton articleform__detailsButton--moreconfig articleform__detailsButton--bottom"></button>
<% end %>
</div>
@ -42,7 +42,7 @@
</div>
</form>
<div style="display:none">
<% if current_user.editor_version == "v2" %>
<% if @user.editor_version == "v2" %>
<%= render "pages/editor_guide_text", version: "2" %>
<% else %>
<%= render "pages/editor_guide_text", version: "1" %>

27
spec/requests/articles_create_spec.rb
View file

@ -1,7 +1,8 @@
require "rails_helper"
RSpec.describe "ArticlesCreate", type: :request do
let(:user) { create(:user) }
let(:organization) { create(:organization) }
let(:user) { create(:user, organization_id: organization.id) }
before do
sign_in user
@ -25,29 +26,15 @@ RSpec.describe "ArticlesCreate", type: :request do
expect(Article.last.title).to eq("hey hey hahuu")
end
it "does not allow job opportunity job to not include hiring tag" do
new_title = "NEW TITLE #{rand(100)}"
expect do
post "/articles", params: {
article: {
title: new_title,
body_markdown: "Yo ho ho#{rand(100)}", tag_list: "yoyo",
job_opportunity: { remoteness: "on_premise" }
}
}
end .to raise_error(RuntimeError)
end
it "creates article with job opportunity nested" do
new_title = "NEW TITLE #{rand(100)}"
it "creates article with front matter params and org" do
post "/articles", params: {
article: {
title: new_title,
body_markdown: "Yo ho ho#{rand(100)}", tag_list: "hiring",
job_opportunity: { remoteness: "on_premise" }
body_markdown: "---\ntitle: hey hey hahuu\npublished: false\n---\nYo ho ho#{rand(100)}",
tag_list: "yo",
post_under_org: true
}
}
expect(Article.last.job_opportunity.remoteness).to eq("on_premise")
expect(Article.last.organization_id).to eq(organization.id)
end
it "creates series when series is created with frontmatter" do

35
spec/requests/articles_update_spec.rb
View file

@ -1,7 +1,10 @@
require "rails_helper"
RSpec.describe "ArticlesUpdate", type: :request do
let(:user) { create(:user) }
let(:organization) { create(:organization) }
let(:organization2) { create(:organization) }
let(:user) { create(:user, organization_id: organization.id) }
let(:user2) { create(:user, organization_id: organization2.id) }
let(:article) { create(:article, user_id: user.id) }
before do
@ -16,25 +19,31 @@ RSpec.describe "ArticlesUpdate", type: :request do
expect(Article.last.title).to eq(new_title)
end
it "does not create a job opportunity if no hiring tag" do
new_title = "NEW TITLE #{rand(100)}"
it "updates article with front matter params" do
put "/articles/#{article.id}", params: {
article: {
title: new_title, body_markdown: "Yo ho ho#{rand(100)}", tag_list: "yo",
job_opportunity: { remoteness: "on_premise" }
title: "hello",
body_markdown: "---\ntitle: hey hey hahuu\npublished: false\n---\nYo ho ho#{rand(100)}",
tag_list: "yo"
}
}
expect(JobOpportunity.count).to eq(0)
expect(Article.last.edited_at).to be > 5.seconds.ago
expect(Article.last.title).to eq("hey hey hahuu")
end
it "updates ordinary article with job opportunity nested" do
new_title = "NEW TITLE #{rand(100)}"
it "adds organization ID when user updates" do
put "/articles/#{article.id}", params: {
article: {
title: new_title, body_markdown: "Yo ho ho#{rand(100)}", tag_list: "hiring",
job_opportunity: { remoteness: "on_premise" }
}
article: { post_under_org: true }
}
expect(Article.last.job_opportunity.remoteness).to eq("on_premise")
expect(article.reload.organization_id).to eq organization.id
end
it "does not modify the organization ID when updating someone else's article as an admin" do
article.update_columns(organization_id: organization2.id, user_id: user2.id)
user.add_role(:super_admin)
put "/articles/#{article.id}", params: {
article: { post_under_org: true }
}
expect(article.reload.organization_id).to eq user2.organization_id
end
end