diff --git a/app/controllers/admin/articles_controller.rb b/app/controllers/admin/articles_controller.rb index 5c8793436..596d82daf 100644 --- a/app/controllers/admin/articles_controller.rb +++ b/app/controllers/admin/articles_controller.rb @@ -40,8 +40,6 @@ module Admin article = Article.find(params[:id]) if article.update(article_params) - PinnedArticle.set(article) if params.dig(:article, :pinned) - flash[:success] = "Article saved!" else flash[:danger] = article.errors_as_sentence @@ -56,7 +54,26 @@ module Admin PinnedArticle.remove respond_to do |format| - format.html { redirect_to admin_article_path(article.id) } + format.html do + flash[:success] = "Article Pinned!" + redirect_to admin_article_path(article.id) + end + format.js do + render partial: "admin/articles/individual_article", locals: { article: article }, content_type: "text/html" + end + end + end + + def pin + article = Article.find(params[:id]) + + PinnedArticle.set(article) + + respond_to do |format| + format.html do + flash[:success] = "Article Pinned!" + redirect_to admin_article_path(article.id) + end format.js do render partial: "admin/articles/individual_article", locals: { article: article }, content_type: "text/html" end diff --git a/app/javascript/admin/controllers/article_controller.js b/app/javascript/admin/controllers/article_controller.js index f43209b28..ac769c91e 100644 --- a/app/javascript/admin/controllers/article_controller.js +++ b/app/javascript/admin/controllers/article_controller.js @@ -32,24 +32,10 @@ export default class ArticleController extends Controller { }, 350); } - togglePin(event) { - const checkbox = event.target; - - // we're only interested in intercepting a checkbox going from - // unchecked to checked - if (!checkbox.checked) { - return; - } - - // by preventing the default, we avoid visually selecting the checkbox, - // it will be responsibility of `pinArticle()` to determine if and when - // the checkbox state has to change + async pinArticle(event) { + const pinArticleForm = event.target; + // We dont want to submit the pin form here. event.preventDefault(); - - this.pinArticle(checkbox); - } - - async pinArticle(checkbox) { const response = await fetch(this.pinPathValue, { method: 'GET', headers: { @@ -75,16 +61,16 @@ export default class ArticleController extends Controller { new CustomEvent('article-pinned-modal:open', { detail: { article: pinnedArticle, - checkboxId: this.pinnedCheckboxTarget.getAttribute('id'), + pinArticleForm, }, }), ); } else { - checkbox.checked = true; + pinArticleForm.submit(); } } else if (response.status === 404) { // if there is no pinned article, it means we can go ahead and pin this one - checkbox.checked = true; + pinArticleForm.submit(); } } diff --git a/app/javascript/admin/controllers/article_pinned_modal_controller.js b/app/javascript/admin/controllers/article_pinned_modal_controller.js index 1b39fcd2a..8ba3634d3 100644 --- a/app/javascript/admin/controllers/article_pinned_modal_controller.js +++ b/app/javascript/admin/controllers/article_pinned_modal_controller.js @@ -3,16 +3,17 @@ import ModalController from './modal_controller'; export default class ArticlePinnedModalController extends ModalController { static targets = ['title', 'pinnedAt', 'pinnedCheckbox']; static values = { - pinnedCheckboxId: String, cancelButtonId: String, okButtonId: String, }; - openModal(event) { - const { article, checkboxId } = event.detail; + pinArticleForm; - // set the caller checkbox ID - this.pinnedCheckboxIdValue = checkboxId; + openModal(event) { + const { article, pinArticleForm } = event.detail; + + // set the pinArticleForm + this.pinArticleForm = pinArticleForm; // update the modal's HTML with the data coming from the server this.titleTarget.setAttribute('href', article.path); @@ -31,21 +32,12 @@ export default class ArticlePinnedModalController extends ModalController { this.toggleModal(); } - changePinnedCheckboxChecked(checked) { - // find the caller checkbox - const pinnedCheckbox = this.pinnedCheckboxTargets.find( - (cb) => cb.id === this.pinnedCheckboxIdValue, - ); - pinnedCheckbox.checked = checked; - } - unPinAndCloseModal() { - this.changePinnedCheckboxChecked(false); this.closeModal(); } pinAndCloseModal() { - this.changePinnedCheckboxChecked(true); + this.pinArticleForm.submit(); this.closeModal(); } } diff --git a/app/views/admin/articles/_individual_article.html.erb b/app/views/admin/articles/_individual_article.html.erb index 908df647d..1a95c6520 100644 --- a/app/views/admin/articles/_individual_article.html.erb +++ b/app/views/admin/articles/_individual_article.html.erb @@ -35,6 +35,14 @@ class: "btn btn-sm btn-secondary", data: { "article-target": "unpinButton" }, ) %> + <% else %> +
+ + +
<% end %> @<%= article.user&.username %> @@ -140,15 +148,9 @@
- <%= check_box_tag :pinned, "1", decorated_article.pinned?, - name: "article[pinned]", - id: "pinned-#{article.id}", - data: { - "article-target": "pinnedCheckbox", - "article-pinned-modal-target": "pinnedCheckbox", - action: "click->article#togglePin" - } %> - + <% if decorated_article.pinned? %> + Pinned Post + <% end %>
diff --git a/config/routes/admin.rb b/config/routes/admin.rb index d3a67ab6b..d98fcd0f2 100644 --- a/config/routes/admin.rb +++ b/config/routes/admin.rb @@ -65,6 +65,7 @@ namespace :admin do resources :articles, only: %i[index show update] do member do delete :unpin + post :pin end end diff --git a/cypress/integration/seededFlows/adminFlows/articles/pinArticle.spec.js b/cypress/integration/seededFlows/adminFlows/articles/pinArticle.spec.js index 7cc16ce74..ce1273dc8 100644 --- a/cypress/integration/seededFlows/adminFlows/articles/pinArticle.spec.js +++ b/cypress/integration/seededFlows/adminFlows/articles/pinArticle.spec.js @@ -30,20 +30,44 @@ describe('Pin an article from the admin area', () => { statusCode: 404, }); - cy.findAllByRole('checkbox', { name: 'Pinned' }).first().check(); - cy.findAllByRole('button', { name: 'Submit' }).first().click(); + cy.findAllByRole('button', { name: 'Pin Post' }).first().click(); - // Verify that the form has submitted and the page has changed to the confirmation page + // Verify that the form has submitted and the page has changed to the post page cy.url().should('contain', '/content_manager/articles/'); - cy.findAllByRole('checkbox', { name: 'Pinned' }) + cy.findByRole('button', { name: 'Pin Post' }).should('not.exist'); + cy.findByRole('link', { name: 'Unpin Post' }).should('exist'); + cy.findByText(/Pinned post/i).should('exist'); + }); + + it('should display a warning modal when pinning an article, and one is already pinned', () => { + cy.findAllByRole('button', { name: 'Pin Post' }).first().click(); + + cy.createArticle({ + title: 'A new article', + tags: ['beginner', 'ruby', 'go'], + content: `This is another test article's contents.`, + published: true, + }).then((response) => { + cy.visit(`/admin/content_manager/articles/${response.body.id}`); + }); + + cy.findByRole('main') .first() - .should('be.checked'); + .within(() => { + cy.findAllByRole('button', { name: 'Pin Post' }).last().click(); + }); + + cy.findByRole('dialog').within(() => { + cy.findByRole('heading', { + name: "There's another article pinned...", + level: 2, + }); + }); }); it('should change the pinned article when choosing to pin a new article', () => { - cy.findAllByRole('checkbox', { name: 'Pinned' }).first().check(); - cy.findAllByRole('button', { name: 'Submit' }).first().click(); + cy.findAllByRole('button', { name: 'Pin Post' }).first().click(); cy.createArticle({ title: 'A new article', @@ -57,47 +81,21 @@ describe('Pin an article from the admin area', () => { cy.findByRole('main') .first() .within(() => { - cy.findAllByRole('checkbox', { name: 'Pinned' }).last().check(); - cy.findAllByRole('button', { name: 'Pin new article' }).last().click(); - cy.findAllByRole('button', { name: 'Submit' }).last().click(); + cy.findAllByRole('button', { name: 'Pin Post' }).last().click(); }); - cy.findByRole('main') - .findAllByRole('checkbox', { name: 'Pinned' }) - .first() - .should('be.checked'); - }); - - it('should not change the pinned article when choosing to dismiss', () => { - cy.findAllByRole('checkbox', { name: 'Pinned' }).first().check(); - cy.findAllByRole('button', { name: 'Submit' }).first().click(); - - cy.createArticle({ - title: 'A new article', - tags: ['beginner', 'ruby', 'go'], - content: `This is another test article's contents.`, - published: true, - }).then((response) => { - cy.visit(`/admin/content_manager/articles/${response.body.id}`); + cy.findByRole('dialog').within(() => { + cy.findByRole('button', { name: 'Pin new article' }).click(); }); - cy.findByRole('main') - .first() - .within(() => { - cy.findAllByRole('checkbox', { name: 'Pinned' }).first().check(); - cy.findAllByRole('button', { name: 'Dismiss' }).first().click(); - cy.findAllByRole('button', { name: 'Submit' }).first().click(); - }); - - cy.findByRole('main') - .findAllByRole('checkbox', { name: 'Pinned' }) - .first() - .should('not.be.checked'); + cy.findByRole('main').within(() => { + cy.findByText(/A new article/i).should('exist'); + cy.findByText(/Pinned post/i).should('exist'); + }); }); it('should show the pinned post to a logged out user', () => { - cy.findAllByRole('checkbox', { name: 'Pinned' }).first().check(); - cy.findAllByRole('button', { name: 'Submit' }).first().click(); + cy.findAllByRole('button', { name: 'Pin Post' }).first().click(); cy.signOutUser(); diff --git a/cypress/integration/seededFlows/adminFlows/articles/unpinArticle.spec.js b/cypress/integration/seededFlows/adminFlows/articles/unpinArticle.spec.js index 473bc23a9..e91eb437f 100644 --- a/cypress/integration/seededFlows/adminFlows/articles/unpinArticle.spec.js +++ b/cypress/integration/seededFlows/adminFlows/articles/unpinArticle.spec.js @@ -16,8 +16,8 @@ describe('Unpin an article from the admin area', () => { tags: ['beginner', 'ruby', 'go'], content: `This is another test article's contents.`, published: true, - }).then((response) => { - cy.visit(`/admin/content_manager/articles/${response.body.id}`); + }).then(() => { + cy.visit('/admin/content_manager/articles'); }); }); }); @@ -25,16 +25,15 @@ describe('Unpin an article from the admin area', () => { }); it('should not display the "Unpin Post" button by default', () => { - cy.findByRole('button', { name: 'Unpin Post' }).should('not.exist'); + cy.findByRole('link', { name: 'Unpin Post' }).should('not.exist'); }); it('should unpin the pinned article', () => { - cy.findAllByRole('checkbox', { name: 'Pinned' }).first().check(); - cy.findAllByRole('button', { name: 'Submit' }).first().click(); + cy.findAllByRole('button', { name: 'Pin Post' }).first().click(); cy.findAllByRole('link', { name: 'Unpin Post' }).first().click(); cy.findAllByRole('link', { name: 'Unpin Post' }).should('not.exist'); - cy.findAllByRole('checkbox', { name: 'Pinned' }).should('not.be.checked'); + cy.findByText(/Pinned post/i).should('not.exist'); }); }); diff --git a/spec/requests/admin/articles_spec.rb b/spec/requests/admin/articles_spec.rb index 643f728bb..333fb65c5 100644 --- a/spec/requests/admin/articles_spec.rb +++ b/spec/requests/admin/articles_spec.rb @@ -53,7 +53,7 @@ RSpec.describe "/admin/content_manager/articles", type: :request do decorated_article = article.decorate expect do - patch admin_article_path(article.id), params: { article: { pinned: true } } + post pin_admin_article_path(article.id) end.to change { decorated_article.pinned? }.to(true) end