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

Migrate Post Form to React #10408

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
6 changes: 0 additions & 6 deletions app/assets/javascripts/posts.js

This file was deleted.

8 changes: 4 additions & 4 deletions app/controllers/posts_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,9 @@ def create
@post.author = current_user
if @post.save
flash[:success] = "Created new post"
redirect_to post_path(@post.slug)
render json: { status: 'ok', post: @post }
else
render 'new'
render json: { status: 'validation failed', errors: @post.errors }, status: :bad_request
end
end

Expand All @@ -80,9 +80,9 @@ def update
@post = find_post
if @post.update(post_params)
flash[:success] = "Updated post"
redirect_to post_path(@post.slug)
render json: { status: 'ok', post: @post }
else
render 'edit'
render json: { status: 'validation failed', errors: @post.errors }, status: :bad_request
end
end

Expand Down
27 changes: 0 additions & 27 deletions app/views/posts/_post_form.html.erb

This file was deleted.

10 changes: 7 additions & 3 deletions app/views/posts/edit.html.erb
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
<% provide(:title, 'Edit post') %>

<div class="container">
<h1><%= yield(:title) %></h1>

<%= render "posts/post_form" %>
<%= react_component("Posts/EditPost", {
allTags: PostTag.distinct.pluck(:tag).map { |tag| { value: tag, text: tag, key: tag } }.as_json,
post: @post.as_json({
only: ["id", "title", "sticky", "show_on_homepage", "unstick_at"],
methods: ["tags_array"],
}),
}) %>
</div>
2 changes: 1 addition & 1 deletion app/views/posts/homepage.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
<div class="column">
<h2 class="ui header"><%= t('homepage.announcements') %></h2>
<div class="ui divider"></div>
<%= react_component("PostsWidget", {
<%= react_component("Posts/PostsWidget", {
titleOnly: true,
}, {
id: "posts_widget",
Expand Down
2 changes: 1 addition & 1 deletion app/views/posts/index.html.erb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<div class="container">
<%= react_component("PostsWidget", {
<%= react_component("Posts/PostsWidget", {
initialPage: @current_page,
}, {
id: "posts_widget",
Expand Down
6 changes: 3 additions & 3 deletions app/views/posts/new.html.erb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<% provide(:title, 'New post') %>

<div class="container">
<h1><%= yield(:title) %></h1>

<%= render "posts/post_form" %>
<%= react_component("Posts/CreatePost", {
allTags: PostTag.distinct.pluck(:tag).map { |tag| { value: tag, text: tag, key: tag } }.as_json,
}) %>
</div>
13 changes: 13 additions & 0 deletions app/webpacker/components/Posts/CreatePost.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import React from 'react';
import WCAQueryClientProvider from '../../lib/providers/WCAQueryClientProvider';
import PostForm from './PostForm';

export default function Wrapper({
allTags,
}) {
return (
<WCAQueryClientProvider>
<PostForm allTags={allTags} header="New Post" />
</WCAQueryClientProvider>
);
}
16 changes: 16 additions & 0 deletions app/webpacker/components/Posts/EditPost.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import React from 'react';
import WCAQueryClientProvider from '../../lib/providers/WCAQueryClientProvider';
import PostForm from './PostForm';
import ConfirmProvider from '../../lib/providers/ConfirmProvider';

export default function Wrapper({
allTags, post,
}) {
return (
<WCAQueryClientProvider>
<ConfirmProvider>
<PostForm post={post} allTags={allTags} header="Edit Post" />
</ConfirmProvider>
</WCAQueryClientProvider>
);
}
141 changes: 141 additions & 0 deletions app/webpacker/components/Posts/PostForm.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
import React, { useCallback, useState } from 'react';
import {
Button, Form, FormField, Header, Message,
} from 'semantic-ui-react';
import { useMutation } from '@tanstack/react-query';
import I18n from '../../lib/i18n';
import useInputState from '../../lib/hooks/useInputState';
import useCheckboxState from '../../lib/hooks/useCheckboxState';
import MarkdownEditor from '../wca/FormBuilder/input/MarkdownEditor';
import { createPost, deletePost, editPost } from './api/posts';
import { useConfirm } from '../../lib/providers/ConfirmProvider';
import UtcDatePicker from '../wca/UtcDatePicker';
import I18nHTMLTranslate from '../I18nHTMLTranslate';

export default function PostForm({
header, allTags, post,
}) {
const [formPost, setFormPost] = useState(post);
const [formTitle, setFormTitle] = useInputState(formPost?.title ?? '');
const [formBody, setFormBody] = useInputState(formPost?.body ?? '');
const [formTags, setFormTags] = useState(formPost?.tags_array ?? []);
const [formIsStickied, setFormIsStickied] = useCheckboxState(formPost?.sticky ?? false);
const [formShowOnHomePage, setFormShowOnHomePage] = useCheckboxState(formPost?.show_on_homepage ?? true);
const [unstickAt, setUnstickAt] = useState(formPost?.unstick_at ?? null);
Comment on lines +18 to +24
Copy link
Contributor

Choose a reason for hiding this comment

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

Feels weird to have states for both the (form)post, and most of the properties of the post. Besides initializing the other states (which happens initially, so doesn't need formPost to be a state), formPost is only used to check whether we're creating or editing, get the id (if we're editing), and show the url at the end.

It seems like we could get rid of formPost and instead just have 2 more states for the post id and url; then update those 2 states in the onSuccess functions below.


const { mutate: createMutation, isSuccess: postCreated, error: createError } = useMutation({
mutationFn: createPost,
onSuccess: ({ data }) => {
setFormPost(data.post);
window.history.replaceState({}, '', `${data.post.url}/edit`);
},
});

const { mutate: editMutation, error: editError, isSuccess: postUpdated } = useMutation({
mutationFn: editPost,
onSuccess: ({ data }) => {
setFormPost(data.post);
},
});

const { errors } = (createError?.json || editError?.json || {});

const onSubmit = useCallback(() => {
if (formPost?.id) {
editMutation({
id: formPost.id,
title: formTitle,
body: formBody,
tags: formTags,
unstick_at: formIsStickied ? unstickAt : null,
sticky: formIsStickied,
show_on_homepage: formShowOnHomePage,
});
} else {
createMutation({
title: formTitle,
body: formBody,
tags: formTags,
sticky: formIsStickied,
unstick_at: formIsStickied ? unstickAt : null,
show_on_homepage: formShowOnHomePage,
});
}
}, [
createMutation,
editMutation,
formBody,
formIsStickied,
formShowOnHomePage,
formTags,
formTitle,
unstickAt,
formPost?.id,
]);

return (
<>
<Header>
{header}
</Header>
<Form onSubmit={onSubmit}>
<FormField>
<Form.Input label={I18n.t('activerecord.attributes.post.title')} onChange={setFormTitle} value={formTitle} />
</FormField>
<FormField>
<label>{I18n.t('activerecord.attributes.post.body')}</label>
<MarkdownEditor onChange={setFormBody} value={formBody} />
{/* i18n-tasks-use t('simple_form.hints.post.body') */}
<I18nHTMLTranslate i18nKey="simple_form.hints.post.body" />
</FormField>
<FormField>
<Form.Select label={I18n.t('activerecord.attributes.post.tags')} options={allTags} onChange={(_, data) => { setFormTags(data.value); }} value={formTags} multiple />
</FormField>
<FormField>
<Form.Checkbox label={I18n.t('activerecord.attributes.post.sticky')} onChange={setFormIsStickied} checked={formIsStickied} />
{ formIsStickied
&& (
<UtcDatePicker
placeholderText={I18n.t('activerecord.attributes.post.unstick_at')}
isoDate={unstickAt}
onChange={(date) => setUnstickAt(date)}
/>
) }
</FormField>
<FormField>
<Form.Checkbox label={I18n.t('activerecord.attributes.post.show_on_homepage')} onChange={setFormShowOnHomePage} checked={formShowOnHomePage} />
<p>{I18n.t('simple_form.hints.post.show_on_homepage')}</p>
</FormField>
{ errors && (
<Message negative>
<Message.Header>Request Failed:</Message.Header>
<Message.List>
{(Object.keys(errors).map((err) => (
<Message.Item>
{err}
{' '}
{errors[err].join(',')}
</Message.Item>
)))}
</Message.List>
</Message>
)}
{ postCreated && (
<Message positive>
Post successfully created. View it
{' '}
<a href={formPost.url}>here</a>
</Message>
)}
{ postUpdated && (
<Message positive>
Post successfully updated. View it
{' '}
<a href={formPost.url}>here</a>
</Message>
)}
Comment on lines +123 to +136
Copy link
Contributor

Choose a reason for hiding this comment

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

If you create a post then edit it, won't both messages be visible?

Side note: Formatting (indentation and spacing) seems to be off here, and I've noticed it in other PRs too.

<Button type="submit" primary>{ formPost ? 'Update Post' : 'Create Post'}</Button>
</Form>
</>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ import {
Button, Card, Icon, List, Pagination,
} from 'semantic-ui-react';

import useLoadedData from '../lib/hooks/useLoadedData';
import { postsUrl } from '../lib/requests/routes.js.erb';
import Loading from './Requests/Loading';
import Errored from './Requests/Errored';
import { formattedTextForDate } from '../lib/utils/wca';
import '../stylesheets/posts_widget.scss';
import useLoadedData from '../../lib/hooks/useLoadedData';
import { postsUrl } from '../../lib/requests/routes.js.erb';
import Loading from '../Requests/Loading';
import Errored from '../Requests/Errored';
import { formattedTextForDate } from '../../lib/utils/wca';
import '../../stylesheets/posts_widget.scss';

function PostTitlesList({
posts,
Expand Down
18 changes: 18 additions & 0 deletions app/webpacker/components/Posts/api/posts.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { fetchJsonOrError } from '../../../lib/requests/fetchWithAuthenticityToken';
import { postUrl, submitPostUrl } from '../../../lib/requests/routes.js.erb';

export const createPost = (post) => fetchJsonOrError(submitPostUrl, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
},
body: JSON.stringify({ post }),
});

export const editPost = (post) => fetchJsonOrError(postUrl(post.id), {
method: 'PATCH',
headers: {
'Content-Type': 'application/json',
},
body: JSON.stringify({ post }),
});
4 changes: 4 additions & 0 deletions app/webpacker/lib/requests/routes.js.erb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ export const postsUrl = (page, format = 'json') => {
return `<%= CGI.unescape(Rails.application.routes.url_helpers.posts_path(format: "${format}")) %>?page=${page}`;
};

export const submitPostUrl = `<%= CGI.unescape(Rails.application.routes.url_helpers.posts_path) %>`;

export const postUrl = (id) => `<%= CGI.unescape(Rails.application.routes.url_helpers.post_path("${id}")) %>`

export const incidentsUrl = (perPage, page, tags = undefined, searchString = undefined, competitions = undefined, format = 'json') => {
const searchParams = new URLSearchParams(`per_page=${perPage}&page=${page}`);
if (tags && tags.length > 0) {
Expand Down
2 changes: 2 additions & 0 deletions config/i18n.yml
Original file line number Diff line number Diff line change
Expand Up @@ -74,4 +74,6 @@ translations:
- "*.attempts.*"
- "*.time_limit.*"
- "*.users.edit.*"
- "*.activerecord.attributes.post.*"
- "*.simple_form.hints.post.*"
- "*.persons.index.*"
4 changes: 0 additions & 4 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -489,10 +489,6 @@ en:
#context: for a post
post:
body: "Use the &lt;!-- break --&gt; tag to show a preview on the home page. Any post on the home page will show all text before this divider. The actual post will display the full body."
sticky: ""
unstick_at: ""
title: ""
tags: ""
show_on_homepage: "Careful! This is not secure for private data. This is only to prevent cluttering the homepage. Posts that are not shown on the homepage are still accessible to the public via permalink or through tags."
#context: for a delegate report
delegate_report:
Expand Down
Loading