-
Notifications
You must be signed in to change notification settings - Fork 794
AO3-6960 Admin post drafts and previews #5822
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
base: master
Are you sure you want to change the base?
Changes from 3 commits
1de91b9
4f509e9
f64b903
812df7c
f568aaf
51e464f
d8befd7
1897463
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,44 +1,51 @@ | ||
| class AdminPostsController < Admin::BaseController | ||
|
|
||
| before_action :admin_only, except: [:index, :show] | ||
| before_action :load_languages, except: [:show, :destroy] | ||
| before_action :load_admin_posts, only: [:index, :drafts] | ||
|
|
||
| # GET /admin_posts | ||
| def index | ||
| if params[:tag] | ||
| @tag = AdminPostTag.find_by(id: params[:tag]) | ||
| if @tag | ||
| @admin_posts = @tag.admin_posts | ||
| end | ||
| end | ||
| @admin_posts ||= AdminPost | ||
| if params[:language_id].present? && (@language = Language.find_by(short: params[:language_id])) | ||
| @admin_posts = @admin_posts.where(language_id: @language.id) | ||
| @tags = AdminPostTag.distinct.joins(:admin_posts).where(admin_posts: { language_id: @language.id }).order(:name) | ||
| else | ||
| @admin_posts = @admin_posts.non_translated | ||
| @tags = AdminPostTag.order(:name) | ||
| end | ||
| @admin_posts = @admin_posts.order('created_at DESC').page(params[:page]) | ||
| @page_subtitle = t(".page_title") | ||
| @admin_posts = @admin_posts.posted.order(published_at: :desc).page(params[:page]) | ||
| end | ||
|
|
||
| # GET /admin_posts/drafts | ||
| def drafts | ||
| authorize AdminPost | ||
|
|
||
| @page_subtitle = t(".page_title") | ||
| @pagy, @admin_posts = pagy(@admin_posts.unposted.order(created_at: :desc)) | ||
| end | ||
|
|
||
| # GET /admin_posts/1 | ||
| def show | ||
| @admin_post = AdminPost.find(params[:id]) | ||
| authorize(@admin_post) unless @admin_post.posted? | ||
|
|
||
| admin_posts = AdminPost.non_translated | ||
| @admin_post = AdminPost.find_by(id: params[:id]) | ||
| unless @admin_post | ||
| raise ActiveRecord::RecordNotFound, "Couldn't find admin post '#{params[:id]}'" | ||
| if @admin_post.posted? | ||
| @admin_posts = admin_posts.posted.order(published_at: :desc).limit(8) | ||
| @previous_admin_post = admin_posts.posted.order(published_at: :desc).where("published_at < ?", @admin_post.published_at).first | ||
| @next_admin_post = admin_posts.posted.order(published_at: :asc).where("published_at > ?", @admin_post.published_at).first | ||
| else | ||
| @admin_posts = admin_posts.unposted.order(created_at: :desc).limit(8) | ||
| @previous_admin_post = admin_posts.unposted.order(created_at: :desc).where("created_at < ?", @admin_post.created_at).first | ||
| @next_admin_post = admin_posts.unposted.order(created_at: :asc).where("created_at > ?", @admin_post.created_at).first | ||
| end | ||
| @admin_posts = admin_posts.order('created_at DESC').limit(8) | ||
| @previous_admin_post = admin_posts.order('created_at DESC').where('created_at < ?', @admin_post.created_at).first | ||
| @next_admin_post = admin_posts.order('created_at ASC').where('created_at > ?', @admin_post.created_at).first | ||
| @page_subtitle = @admin_post.title.html_safe | ||
| respond_to do |format| | ||
| format.html # show.html.erb | ||
| format.js | ||
| end | ||
| end | ||
|
|
||
| # GET /admin_posts/1/preview | ||
| def preview | ||
| @preview_mode = true | ||
| @admin_post = AdminPost.find(params[:id]) | ||
| authorize(@admin_post) | ||
| end | ||
|
|
||
| # GET /admin_posts/new | ||
| # GET /admin_posts/new.xml | ||
| def new | ||
|
|
@@ -55,10 +62,15 @@ def edit | |
| # POST /admin_posts | ||
| def create | ||
| @admin_post = AdminPost.new(admin_post_params) | ||
| @admin_post.posted = true if params[:post_button] && !@admin_post&.translated_post&.draft? | ||
|
|
||
| authorize @admin_post | ||
| if @admin_post.save | ||
| flash[:notice] = ts("Admin Post was successfully created.") | ||
| redirect_to(@admin_post) | ||
| if params[:preview_button] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we validate that the post is valid before previewing? Something similar to what's done in |
||
| @preview_mode = true | ||
| render action: "preview" | ||
| elsif !params[:edit_button] && @admin_post.save | ||
| flash[:notice] = t(".success") | ||
| redirect_to(admin_post_path(@admin_post)) | ||
| else | ||
| render action: "new" | ||
| end | ||
|
|
@@ -67,12 +79,36 @@ def create | |
| # PUT /admin_posts/1 | ||
| def update | ||
| @admin_post = AdminPost.find(params[:id]) | ||
| @admin_post.attributes = admin_post_params | ||
| @admin_post.posted = true if params[:post_button] && !@admin_post&.translated_post&.draft? | ||
| authorize @admin_post | ||
| if @admin_post.update(admin_post_params) | ||
| flash[:notice] = ts("Admin Post was successfully updated.") | ||
| redirect_to(@admin_post) | ||
|
|
||
| if !params[:edit_button] && @admin_post.valid? | ||
| if params[:preview_button] | ||
| @preview_mode = true | ||
| render :preview and return | ||
| elsif @admin_post.save | ||
| flash[:notice] = t(".success") | ||
| redirect_to(@admin_post) and return | ||
| end | ||
| end | ||
|
|
||
| render action: "edit" | ||
| end | ||
|
|
||
| # PUT /admin_posts/1/post | ||
| def post | ||
| @admin_post = AdminPost.find(params[:id]) | ||
| authorize @admin_post | ||
|
|
||
| @admin_post.posted = true | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we also check
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've added a validation to the model to prevent translations from being posted first, which should cover this and also prevent already published posts from turned into translations of a draft post.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great approach, thanks for taking care of that! |
||
|
|
||
| if @admin_post.save | ||
| flash[:notice] = t(".success") | ||
| redirect_to @admin_post | ||
| else | ||
| render action: "edit" | ||
| flash[:error] = t(".error") | ||
| redirect_to(edit_admin_post_path(@admin_post)) | ||
| end | ||
| end | ||
|
|
||
|
|
@@ -81,7 +117,8 @@ def destroy | |
| @admin_post = AdminPost.find(params[:id]) | ||
| authorize @admin_post | ||
| @admin_post.destroy | ||
| redirect_to(admin_posts_path) | ||
|
|
||
| redirect_to(@admin_post.posted? ? admin_posts_path : drafts_admin_posts_path) | ||
| end | ||
|
|
||
| protected | ||
|
|
@@ -90,6 +127,19 @@ def load_languages | |
| @news_languages = Language.where(id: Locale.all.map(&:language_id)).default_order | ||
| end | ||
|
|
||
| def load_admin_posts | ||
| @tag = AdminPostTag.find_by(id: params[:tag]) if params[:tag] | ||
| @admin_posts = @tag&.admin_posts || AdminPost | ||
|
|
||
| if params[:language_id].present? && (@language = Language.find_by(short: params[:language_id])) | ||
| @admin_posts = @admin_posts.where(language_id: @language.id) | ||
| @tags = AdminPostTag.distinct.joins(:admin_posts).where(admin_posts: { language_id: @language.id }).order(:name) | ||
| else | ||
| @admin_posts = @admin_posts.non_translated | ||
| @tags = AdminPostTag.order(:name) | ||
| end | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def admin_post_params | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,25 @@ | ||
| class AdminPostPolicy < ApplicationPolicy | ||
| POSTING_ROLES = %w[superadmin board board_assistants_team communications support translation].freeze | ||
| DRAFTING_ROLES = %w[policy_and_abuse].freeze | ||
|
|
||
| def can_post? | ||
| user_has_roles?(POSTING_ROLES) | ||
| end | ||
|
|
||
| alias new? can_post? | ||
| alias edit? can_post? | ||
| alias create? can_post? | ||
| alias update? can_post? | ||
| alias destroy? can_post? | ||
| def can_draft? | ||
| user_has_roles?(DRAFTING_ROLES) || can_post? | ||
| end | ||
|
|
||
| def edit? | ||
| can_post? || (@record&.draft? && can_draft?) | ||
| end | ||
|
|
||
| alias new? can_draft? | ||
| alias show? can_draft? | ||
| alias create? edit? | ||
| alias update? edit? | ||
| alias destroy? edit? | ||
| alias post? can_post? | ||
| alias drafts? can_draft? | ||
| alias preview? edit? | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,10 +3,17 @@ | |
| <li> | ||
| <%= span_if_current t(".ao3_news"), admin_posts_path %> | ||
| </li> | ||
| <li> | ||
| <%= span_if_current t(".ao3_news_drafts"), drafts_admin_posts_path %> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The "AO3 News Drafts" link is shown without a permission check. The ticket says it should be visible to "any admin with a role listed in the AdminPostPolicy section." Should this be wrapped in |
||
| </li> | ||
| <% if policy(AdminPost).can_post? %> | ||
| <li> | ||
| <%= span_if_current t(".post_ao3_news"), new_admin_post_path %> | ||
| </li> | ||
| <% elsif policy(AdminPost).can_draft? %> | ||
| <li> | ||
| <%= span_if_current t(".draft_ao3_news"), new_admin_post_path %> | ||
| </li> | ||
| <% end %> | ||
| <% if params[:controller] == "admin_posts" && params[:action] == "edit" %> | ||
| <li> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,27 +1,17 @@ | ||
| <!--Descriptive page name, messages and instructions--> | ||
| <h2 class="heading"><%= ts("AO3 News (includes Release Notes)") %></h2> | ||
| <h2 class="heading"><%= t(".page_heading") %></h2> | ||
| <!--/descriptions--> | ||
| <!--subnav--> | ||
| <%= render 'filters' %> | ||
| <%= render :partial => 'admin/admin_nav' %> | ||
| <!--/subnav--> | ||
|
|
||
| <!--main content--> | ||
| <h3 class="heading"><%= ts("Manage News Postings") %></h3> | ||
| <h3 class="landmark heading"><%= t(".landmark") %></h3> | ||
| <dl class="news index group"> | ||
| <% @admin_posts.each do |admin_post| %> | ||
| <dt><%= link_to admin_post.title.html_safe, admin_post %></dt> | ||
| <dd> | ||
| <p class="datetime"><%= ts("Created at %{created_date} and updated at %{updated_date}", :created_date => admin_post.created_at, :updated_date => admin_post.updated_at) %></p> | ||
| <ul class="actions"> | ||
| <li><%= link_to ts("Show"), admin_post %></li> | ||
| <li><%= link_to ts("Edit"), edit_admin_post_path(admin_post) %></li> | ||
| <li><%= link_to ts("Delete"), admin_post, data: {confirm: 'Are you sure?'}, :method => :delete %></li> | ||
| </ul> | ||
| </dd> | ||
| <% end %> | ||
| <%= render partial: "admin_post_blurb", collection: @admin_posts, as: :admin_post %> | ||
| </dl> | ||
| <!--/content--> | ||
| <!--subnav--> | ||
| <%= will_paginate @admin_posts %> | ||
| <%= will_paginate @admin_posts %> | ||
| <!--/subnav--> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| <%# expects admin_post %> | ||
| <dt><%= link_to admin_post.title.html_safe, admin_post %></dt> | ||
| <dd> | ||
| <p class="datetime"> | ||
| <% if admin_post.posted? %> | ||
| <%= t(".meta.published", publication_date: admin_post.published_at, created_date: admin_post.created_at, updated_date: admin_post.updated_at) %> | ||
| <% else %> | ||
| <%= t(".meta.draft", created_date: admin_post.created_at, updated_date: admin_post.updated_at) %> | ||
| <% end %> | ||
| </p> | ||
| <ul class="actions"> | ||
| <li><%= link_to t(".show"), admin_post %></li> | ||
| <% if policy(admin_post).edit? %> | ||
| <li><%= link_to t(".edit"), edit_admin_post_path(admin_post) %></li> | ||
| <% if admin_post.posted %> | ||
| <li><%= link_to t(".delete"), admin_post, data: { confirm: t(".confirm_delete") }, method: :delete %></li> | ||
| <% else %> | ||
| <% if policy(AdminPost).can_post? && (admin_post.translated_post.blank? || admin_post.translated_post.posted?) %> | ||
| <li><%= link_to t(".draft.post"), post_admin_post_path(admin_post), method: :put %></li> | ||
| <% end %> | ||
| <li><%= link_to t(".draft.delete"), admin_post, data: { confirm: t(".confirm_delete") }, method: :delete %></li> | ||
| <% end %> | ||
| <% end %> | ||
| </ul> | ||
| </dd> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This uses will_paginate, but drafts below uses pagy. Should this also use pagy for consistency?