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

COR-710: Publish System Rebuild #487

Merged
merged 15 commits into from
Apr 26, 2017
Merged
Show file tree
Hide file tree
Changes from 10 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
15 changes: 1 addition & 14 deletions app/assets/javascripts/datepicker_init.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,7 @@ var datepicker_init = function() {
if (datepicker.length) {
datepicker.datetimepicker({
dateFormat: 'dd/mm/yy',
onSelect: function (date) {
Copy link
Member

Choose a reason for hiding this comment

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

We still need to reflect the latest state as the user changes publish/expiration dates, as per Justin's previous AC

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I talked with him about this, since the buttons wouldn't actually move the state along he preferred if it just said "Save Now" until we figure out how best to implement the triggered state changes

var button = $('.new_publish_state_button');
var currentDate = new Date();
var selectedDate = moment(date, 'DD-MM-YYYY HH:mm');

if (button.length > 0 && selectedDate > currentDate) {
button.val('schedule');
button.text('Schedule Post');
}
else if (button.length > 0 && selectedDate <= currentDate) {
button.val('published');
button.text('Publish Now');
}
}
timeFormat: 'HH:mm Z'
});

datepicker.on('focusout', function(ev){
Expand Down
7 changes: 0 additions & 7 deletions app/jobs/publish_content_item_job.rb

This file was deleted.

25 changes: 3 additions & 22 deletions app/models/content_item.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,8 @@ class ContentItem < ApplicationRecord
include Elasticsearch::Model
include Elasticsearch::Model::Callbacks

state_machine :initial => :default do
state_machine do
state :draft
state :scheduled, :enter => lambda { |content_item| content_item.schedule_publish }
state :published
state :default #the default state that is given to an object - this should only ever exist on ContentItems where the ContentType is not publishable

event :publish do
transitions :to => :published, :from => [:default, :draft, :scheduled]
end

event :schedule do
transitions :to => :scheduled, :from => [:default, :draft]
end

event :draft do
transitions :to => :draft, :from => [:default, :published, :scheduled]
end
end

acts_as_paranoid
Expand Down Expand Up @@ -47,12 +32,8 @@ def author_email
end

def publish_state
state.titleize
end

def schedule_publish
timestamp = field_items.find { |field_item| field_item.field.name == "Publish Date" }.data["timestamp"]
PublishContentItemJob.set(wait_until: DateTime.parse(timestamp)).perform_later(self)
sorted_field_items = field_items.select { |fi| fi.field.field_type_instance.is_a?(DateTimeFieldType) && !fi.field.metadata["state"].nil? }.sort_by{ |a| a.data["timestamp"] }.reverse
Copy link
Member

Choose a reason for hiding this comment

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

I have some thoughts about this system (which is pretty neat!), but we may end up rebuilding some of this anyway - so we can discuss later.

Copy link
Member

Choose a reason for hiding this comment

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

Let's move this logic to the Service layer - consumers of this abstraction ideally shouldn't have to know how to groom the data being passed in. This issue of 'leaky abstractions' is something that I'm trying to correct across the application - it's hard to stay ontop of, and it's something we've all been guilty of. Thanks!

PublishStateService.new.perform(sorted_field_items, self)
end

def rss_url(base_url, slug_field_id)
Expand Down
2 changes: 1 addition & 1 deletion app/services/content_item_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ def update

transact_and_refresh do
@content_item.update(content_item_attributes)
PublishStateService.clear(@content_item)
end
end

Expand All @@ -55,7 +56,6 @@ def transact_and_refresh
yield
parse_field_items!
@content_item.save!
execute_state_change(@content_item)
update_search!
end
end
Expand Down
27 changes: 27 additions & 0 deletions app/services/publish_state_service.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
class PublishStateService < ApplicationController
Copy link
Member

Choose a reason for hiding this comment

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

Let's not inherit from ApplicationController here - this brings in a lot of stuff. We can follow our existing service data object pattern - inherit from ApplicationService and specify your Service's attributes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops - that's a typo, good catch

def perform(sorted_field_items, content_item)
Copy link
Member

Choose a reason for hiding this comment

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

I'm guessing this returns the publish status of a ContentItem - if so, we'll want to make the method name reflect that

@sorted_field_items = sorted_field_items
@content_item = content_item

Rails.cache.fetch("PublishStateService/#{content_item.id}", expires_in: 10.minutes) do
active_state
end
end

def self.clear(content_item)
Rails.cache.delete("PublishStateService/#{content_item.id}")
end

private

def active_state
Copy link
Member

Choose a reason for hiding this comment

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

Since our terminology is 'published', should this be active_state?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Our terminology is only published for this specific ContentType, this method is only trying to figure out which, of the potentially multiple states, is presently active

@sorted_field_items.each do |field_item|
if DateTime.now > DateTime.parse(field_item.data["timestamp"])
return field_item.field.metadata["state"]
break
end
end

return @content_item.state.titleize
end
end
15 changes: 1 addition & 14 deletions app/views/content_items/_form.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,5 @@
= cell('wizard', @wizard, context: { content_item: @content_item, form: form }).()
%footer.mdl-grid
.mdl-cell.mdl-cell--12-col
- if @content_type.publishable
- case @content_item.state
- when 'published'
= form.submit "Update Post", class: 'mdl-button mdl-js-button mdl-button--raised mdl-button--success mdl-js-ripple-effect'
= form.button 'Save as Draft', value: 'draft', name: 'content_item[state]', class: 'mdl-button mdl-js-button mdl-button--cb mdl-js-ripple-effect', type: 'submit'
- when 'scheduled'
= form.button 'Schedule Post', value: 'schedule', name: 'content_item[state]', class: 'mdl-button mdl-js-button mdl-button--cb mdl-js-ripple-effect', type: 'submit'
= form.button 'Publish Now', value: 'publish', name: 'content_item[state]', class: 'mdl-button mdl-js-button mdl-button--raised mdl-button--success mdl-js-ripple-effect', type: 'submit'
= form.button 'Save as Draft', value: 'draft', name: 'content_item[state]', class: 'mdl-button mdl-js-button mdl-button--cb mdl-js-ripple-effect', type: 'submit'
- else
= form.button 'Publish Now', value: 'publish', name: 'content_item[state]', class: 'mdl-button mdl-js-button mdl-button--raised mdl-button--success mdl-js-ripple-effect new_publish_state_button', type: 'submit'
= form.button 'Save as Draft', value: 'draft', name: 'content_item[state]', class: 'mdl-button mdl-js-button mdl-button--cb mdl-js-ripple-effect', type: 'submit'
- else
= form.submit 'Submit', class: 'mdl-button mdl-js-button mdl-button--raised mdl-button--success mdl-js-ripple-effect'
= form.submit 'Save Now', class: 'mdl-button mdl-js-button mdl-button--raised mdl-button--success mdl-js-ripple-effect'
Copy link
Member

Choose a reason for hiding this comment

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

I believe we'll still need to reflect the state in the button's text, per Justin's previous AC

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I talked with him about this, since the buttons wouldn't actually move the state along he preferred if it just said "Save Now" until we figure out how best to implement the triggered state changes

= link_to 'Cancel', content_type_content_items_path(@content_type), class: 'mdl-button mdl-js-button mdl-button--cb mdl-js-ripple-effect'
14 changes: 10 additions & 4 deletions lib/tasks/employer/blog.rake
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@ namespace :employer do

def category_tree
tree = Tree.new
tree.add_node({ name: "Recruitment Solutions" })
tree.add_node({ name: "Product News" })
tree.add_node({ name: "Company News, Research and Trends" })
tree.add_node({ name: "Client Success Stories" })
tree.add_node({ name: "Recruiting Solutions" })
tree.add_node({ name: "Employment Screening" })
tree.add_node({ name: "Human Capital Management" })

Expand Down Expand Up @@ -65,8 +68,8 @@ namespace :employer do
blog.fields.new(name: 'Slug', field_type: 'text_field_type', validations: {presence: true, uniqueness: true, length: { maximum: 75, minimum: 30} })
blog.fields.new(name: 'Author', field_type: 'author_field_type')
blog.fields.new(name: 'Tags', field_type: 'tag_field_type')
blog.fields.new(name: 'Publish Date', field_type: 'date_time_field_type')
blog.fields.new(name: 'Expiration Date', field_type: 'date_time_field_type')
blog.fields.new(name: 'Publish Date', field_type: 'date_time_field_type', metadata: {state: 'Published'})
blog.fields.new(name: 'Expiration Date', field_type: 'date_time_field_type', metadata: {state: 'Expired'})
blog.fields.new(name: 'SEO Title', field_type: 'text_field_type', validations: {presence: true, length: {maximum: 70}, uniqueness: true })
blog.fields.new(name: 'SEO Description', field_type: 'text_field_type', validations: {presence: true, length: {maximum: 160} })
blog.fields.new(name: 'SEO Keywords', field_type: 'tag_field_type')
Expand Down Expand Up @@ -140,7 +143,7 @@ namespace :employer do
"id": blog.fields.find_by_name('Publish Date').id
},
{
"id": blog.fields.find_by_name('Author').id
"id": blog.fields.find_by_name('Expiration Date').id
}
]
},
Expand All @@ -153,6 +156,9 @@ namespace :employer do
{
"id": blog.fields.find_by_name('Slug').id,
"tooltip": "This is your post's URL. Between each word, place a hyphen. Best if between 35-50 characters and don't include years/dates."
},
{
"id": blog.fields.find_by_name('Author').id
}
]
}
Expand Down