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

Fix: cannot unpin articles using checkbox (#15090)

Browse source 
Co-authored-by: rhymes <github@rhymes.dev>
Co-authored-by: Arit Amana <msarit@gmail.com>
This commit is contained in:
Kushal Niroula 2021-11-02 23:35:56 +05:45 • committed by GitHub
parent da36b5fd17
commit 87244c345c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 90 additions and 95 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

23
app/controllers/admin/articles_controller.rb
View file

@ -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

26
app/javascript/admin/controllers/article_controller.js
View file

@ -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();
}
}

22
app/javascript/admin/controllers/article_pinned_modal_controller.js
View file

@ -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();
}
}

20
app/views/admin/articles/_individual_article.html.erb
View file

@ -35,6 +35,14 @@
class: "btn btn-sm btn-secondary",
data: { "article-target": "unpinButton" },
) %>
<% else %>
<form method="post" action="<%= pin_admin_article_path(decorated_article.id) %>" class="inline"
data-action="submit->article#pinArticle">
<input type="hidden" name="authenticity_token" value="<%= form_authenticity_token %>" />
<button type="submit" class="btn btn-sm btn-secondary">
Pin Post
</button>
</form>
<% end %>
<a href="<%= admin_user_path(article.user_id) %>" class="badge badge-light">@<%= article.user&.username %></a>
@ -140,15 +148,9 @@
</div>
<div class="form-check col">
<div class="form-check col">
<%= 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"
} %>
<label for="pinned-<%= article.id %>">Pinned</label>
<% if decorated_article.pinned? %>
<span class="badge badge-success">Pinned Post</span>
<% end %>
</div>
<div class="col">
<button class="btn btn-primary float-right">Submit</button>

1
config/routes/admin.rb
View file

@ -65,6 +65,7 @@ namespace :admin do
resources :articles, only: %i[index show update] do
member do
delete :unpin
post :pin
end
end

80
cypress/integration/seededFlows/adminFlows/articles/pinArticle.spec.js
View file

@ -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();

11
cypress/integration/seededFlows/adminFlows/articles/unpinArticle.spec.js
View file

@ -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');
});
});

2
spec/requests/admin/articles_spec.rb
View file

@ -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