Create abaquery forum #4446

jonasdeluna · 2024-02-06T19:14:41Z

Description

Create abaquery forum, now working MVP.
There is still more I want to add, refer to linear. For now this is a very basic working forum (Like the notification on @ someone, subscribe to threads, and better content options beside just text)

Styling is not my strong suit, so all the sideborder colors on the threads are blue, which is kind of ugly, but I wasn't sure what to put because it should probably be different from the forums.
Maybe the color can be defined for each thread/forum? But that could also create a lot of chaos.

For now I added it to the hamburger menu. Maybe it should be more visible like in the top bar, but we also don't want too much clutter there.

I added a hover effect on comments that shows when they were created:

(If you're wondering why I'm using edge as my dev browser now it's because all other browsers for some reason doesn't work with cookies so I get auto-logged out and everything breaks)

I think this is a good opportunity to allow others to in the very least remove other comments with enough permissions, maybe restricted to forums but I could see it useful for articles as well.

Thread creation is IsAuthenticated, forum creation is some sort of admin (ref lego PR)
As per now, I believe everyone has the actiongrant for thread.create and thread.edit because everyone is allowed to create threads, and edit their own. Not sure how to perform a check for permission to delete/edit others' threads or if this should be a thing. I feel like someone will definitely shitpost and it needs to be deleted so having to shell would be a PITA.

Also adding reactions to comments could be a quick way to implement the karma system (more about this on linear) @haukkagu

Depends on: webkom/lego#3549

*EDIT: Update the content of a thread to be a lego-editor content. Not sure about opinions on this, but at least the content is formattable now.

Result

Changes look good on both light and dark theme.
Changes look good with different viewports (mobile, tablet, etc.).
Changes look good with slower Internet connections.

Caution

Make sure your images do not contain any real user information.

Description	Before	After
...	...	...

Testing

I have thoroughly tested my changes.

Please describe what and how the changes have been tested, and provide instructions to reproduce if necessary.

Resolves ABA-561

linear · 2024-02-07T21:03:00Z

ABA-561 Create abaquery forum

ivarnakken · 2024-02-14T17:34:00Z

Would you like any reviews on this while it is still a draft?

jonasdeluna · 2024-02-14T17:37:40Z

Would you like any reviews on this while it is still a draft?

If you have any feedback already that would be appreciated, though I'm working on finishing an MVP on this soon so if you haven't read through it I think it would be better to review after the MVP 👍

ivarnakken

Very interesting feature! Good job! 🏅

We should perhaps consider just calling this "Forum" instead of "AbaQuery" though.

app/store/createRootReducer.ts

app/routes/forum/components/ThreadEditor.tsx

app/routes/forum/components/ThreadDetail.tsx

ivarnakken · 2024-02-19T07:28:14Z

app/routes/forum/components/ThreadDetail.tsx

+
+  return (
+    thread && (
+      <LoadingIndicator loading={fetchingThreads && fetchingComments}>


No need to show loading indicator if threads or comments is not empty in the redux store. Consider using skeleton components <3

Okay I'll have a look after the other things are resolved on how to implement this.

app/routes/forum/components/ThreadList.tsx

app/routes/forum/components/ThreadEditor.tsx

app/routes/forum/components/ForumListEntry.tsx

ivarnakken · 2024-02-19T08:13:54Z

I realize that I made a lot of comments - so don't feel like you have to resolve all of them. Feel free to iterate on it in a new PR later 😄

jonasdeluna · 2024-02-19T12:11:58Z

I realize that I made a lot of comments - so don't feel like you have to resolve all of them. Feel free to iterate on it in a new PR later 😄

Ill go through and try to fix them 👍
What do you think about changing description to a Lego editor for more formatting on the body of a thread. I pushed it but can be reverted if it's not wanted

jonasdeluna · 2024-02-20T19:49:37Z

I rebased and fixed your comments - updated screenshots @ivarnakken
I think it's good for a MVP.
Another sort of important thing I want to add is stickying but we can also test this in staging perhaps before that...
Or should it get more features before pushing out?

ivarnakken · 2024-02-23T14:29:34Z

What do you think about changing description to a Lego editor for more formatting on the body of a thread. I pushed it but can be reverted if it's not wanted

I like that!

Another sort of important thing I want to add is stickying but we can also test this in staging perhaps before that...
Or should it get more features before pushing out?

This is a great MVP! If you want we can probably merge this in 😄 I'd love to get some more views on this new feature though.

Are you planning on migrating threads from the slack channel? I believe we have saved most (or a lot) through the Matrix clone

jonasdeluna · 2024-02-23T15:30:28Z

This is a great MVP! If you want we can probably merge this in 😄 I'd love to get some more views on this new feature though.

Are you planning on migrating threads from the slack channel? I believe we have saved most (or a lot) through the Matrix clone

Alright, backend is pushed and should be good.
Sure we could try migrating them if we have an export already.
The structure is a little different though, as a "thread" here is not a slack-channel but maybe its actually 1:1?
Forum - Channel
Message/Thread - Thread?
I think we can do it after merging in so case?

norbye

Sorry for reviewing a little late - but here we are(:

Ended up being a few more comments than I planned for, and as this is becoming a kinda chunky PR I'll leave it as approved if you want to merge it as the first iterations. But if you have the time it would be awesome if you just skim over the comments to see if we agree

Edit: one of the things you might want to look at before merging anyways are the changes you made to components under the articles route - but as long as your changes are unique to the forums functionality I'd say go for it if you want to

norbye · 2024-02-24T22:17:24Z

app/actions/ForumActions.ts

+    schema: forumSchema,
+    meta: {
+      errorMessage: 'Opprettelse av forum feilet',
+    },


Is there any reason why you only added requiresAuthentication to the first two endpoints?

norbye · 2024-02-24T22:18:14Z

app/actions/ForumActions.ts

+export function fetchThreads() {
+  return callAPI<PublicThread[]>({
+    types: Thread.FETCH_ALL,
+    endpoint: '/threads/',
+    schema: [threadSchema],
+    method: 'GET',
+    meta: {
+      errorMessage: 'Henting av tråder feilet',
+    },
+    propagateError: true,
+  });
+}


Delete this?

norbye · 2024-02-24T22:18:43Z

app/actions/ForumActions.ts

+export function fetchThread(threadId: ID) {
+  return callAPI<DetailedThread>({
+    types: Thread.FETCH,
+    endpoint: `/threads/${threadId}/`,
+    schema: threadSchema,
+    meta: {
+      errorMessage: 'Henting av tråd feilet',
+    },
+    propagateError: true,
+  });
+}


Delete this?

norbye · 2024-02-24T22:19:56Z

app/actions/ForumActions.ts

+export function createThread(thread: CreateThread) {
+  return callAPI<DetailedThread>({
+    types: Thread.CREATE,
+    endpoint: '/threads/',


Suggested change

endpoint: '/threads/',

endpoint: `/forums/${forumId}/threads/`,

norbye · 2024-02-24T22:20:12Z

app/actions/ForumActions.ts

+export function editThread(thread: UpdateThread) {
+  return callAPI({
+    types: Thread.UPDATE,
+    endpoint: `/threads/${thread.id}/`,


Suggested change

endpoint: `/threads/${thread.id}/`,

endpoint: `/forums/${forumId}/threads/${threadId}`,

norbye · 2024-02-24T22:40:42Z

app/routes/forum/components/ThreadDetail.tsx

+            {(thread.createdBy?.id === currentUser.id ||
+              detailActionGrant.includes('edit')) && (


If I understood your backend-changes correctly, actionGrant should include the first line if it matches the permissions.py thingy, which I hope it does

Suggested change

{(thread.createdBy?.id === currentUser.id ||

detailActionGrant.includes('edit')) && (

{detailActionGrant.includes('edit')) && (

norbye · 2024-02-24T22:41:35Z

app/routes/forum/components/ThreadEditor.tsx

+  const { threadId } = useParams<{ threadId: string }>();
+  const { forumId } = useParams<{ forumId: string }>();


Suggested change

const { threadId } = useParams<{ threadId: string }>();

const { forumId } = useParams<{ forumId: string }>();

const { forumId, threadId } = useParams<{ forumId: string, threadId: string }>();

app/routes/forum/components/ThreadEditor.tsx

norbye · 2024-02-24T22:46:47Z

app/routes/forum/index.tsx

+const ForumRoute = () => (
+  <Routes>
+    <Route index element={<ForumList />} />
+    <Route path=":forumId/threads" element={<ForumDetail />} />


Now I'm knitpicking at a pretty serious level - but this feels more natural idk

Suggested change

<Route path=":forumId/threads" element={<ForumDetail />} />

<Route path=":forumId" element={<ForumDetail />} />

norbye · 2024-02-24T22:47:14Z

app/routes/forum/index.tsx

+    <Route path=":forumId/threads" element={<ForumDetail />} />
+    <Route path="new" element={<ForumEditor />} />
+    <Route path=":forumId/edit" element={<ForumEditor />} />
+    <Route path=":forumId/new" element={<ThreadEditor />} />


Suggested change

<Route path=":forumId/new" element={<ThreadEditor />} />

<Route path=":forumId/threads/new" element={<ThreadEditor />} />

jonasdeluna · 2024-02-26T16:51:01Z

As mentioned in the backend PR I think I'll merge this for now and resolve all the other comments in the next sprint.

jonasdeluna added the new-feature Pull requests that introduce a new feature label Feb 6, 2024

github-actions bot added the review-needed Pull requests that need review label Feb 6, 2024

jonasdeluna force-pushed the create-abaquery-forum branch from e75a62c to a401cf3 Compare February 15, 2024 20:38

jonasdeluna marked this pull request as ready for review February 18, 2024 21:42

jonasdeluna requested review from ivarnakken and eikhr February 18, 2024 21:43

jonasdeluna self-assigned this Feb 18, 2024

ivarnakken requested changes Feb 19, 2024

View reviewed changes

ivarnakken requested a review from a team February 19, 2024 07:58

ivarnakken added the changes-requested Pull requests with requested changes label Feb 19, 2024

jonasdeluna force-pushed the create-abaquery-forum branch from 41049f7 to edbfeb9 Compare February 20, 2024 17:36

Create the abaquery forum in front-end

0d300cd

jonasdeluna force-pushed the create-abaquery-forum branch from feb49b8 to 0d300cd Compare February 20, 2024 18:56

jonasdeluna requested a review from ivarnakken February 20, 2024 19:10

Start adding sticky

e792a11

jonasdeluna force-pushed the create-abaquery-forum branch 2 times, most recently from caab7ea to e0f8249 Compare February 23, 2024 15:23

Update endpoints to reflect backend structure

8ab8093

jonasdeluna force-pushed the create-abaquery-forum branch from e0f8249 to 8ab8093 Compare February 23, 2024 15:25

norbye approved these changes Feb 24, 2024

View reviewed changes

jonasdeluna force-pushed the create-abaquery-forum branch 2 times, most recently from 69a10db to 4b2a2ec Compare February 26, 2024 16:49

Update actiongrants and minimize fetching

a21b0d2

jonasdeluna force-pushed the create-abaquery-forum branch from 4b2a2ec to a21b0d2 Compare February 26, 2024 16:52

jonasdeluna merged commit 0d01ead into master Feb 26, 2024
4 checks passed

jonasdeluna deleted the create-abaquery-forum branch February 26, 2024 22:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create abaquery forum #4446

Create abaquery forum #4446

jonasdeluna commented Feb 6, 2024 •

edited

linear bot commented Feb 7, 2024

ivarnakken commented Feb 14, 2024

jonasdeluna commented Feb 14, 2024

ivarnakken left a comment

ivarnakken Feb 19, 2024

jonasdeluna Feb 19, 2024

ivarnakken commented Feb 19, 2024

jonasdeluna commented Feb 19, 2024

jonasdeluna commented Feb 20, 2024 •

edited

ivarnakken commented Feb 23, 2024

jonasdeluna commented Feb 23, 2024

norbye left a comment •

edited

norbye Feb 24, 2024

norbye Feb 24, 2024

norbye Feb 24, 2024

norbye Feb 24, 2024

norbye Feb 24, 2024

norbye Feb 24, 2024

norbye Feb 24, 2024

norbye Feb 24, 2024

norbye Feb 24, 2024

jonasdeluna commented Feb 26, 2024

	endpoint: '/threads/',
	endpoint: `/forums/${forumId}/threads/`,

	endpoint: `/threads/${thread.id}/`,
	endpoint: `/forums/${forumId}/threads/${threadId}`,

		{(thread.createdBy?.id === currentUser.id \|\|
		detailActionGrant.includes('edit')) && (

	{(thread.createdBy?.id === currentUser.id \|\|
	detailActionGrant.includes('edit')) && (
	{detailActionGrant.includes('edit')) && (

		const { threadId } = useParams<{ threadId: string }>();
		const { forumId } = useParams<{ forumId: string }>();

	const { threadId } = useParams<{ threadId: string }>();
	const { forumId } = useParams<{ forumId: string }>();
	const { forumId, threadId } = useParams<{ forumId: string, threadId: string }>();

	<Route path=":forumId/threads" element={<ForumDetail />} />
	<Route path=":forumId" element={<ForumDetail />} />

	<Route path=":forumId/new" element={<ThreadEditor />} />
	<Route path=":forumId/threads/new" element={<ThreadEditor />} />

Create abaquery forum #4446

Create abaquery forum #4446

Conversation

jonasdeluna commented Feb 6, 2024 • edited

Description

Result

Testing

linear bot commented Feb 7, 2024

ivarnakken commented Feb 14, 2024

jonasdeluna commented Feb 14, 2024

ivarnakken left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ivarnakken commented Feb 19, 2024

jonasdeluna commented Feb 19, 2024

jonasdeluna commented Feb 20, 2024 • edited

ivarnakken commented Feb 23, 2024

jonasdeluna commented Feb 23, 2024

norbye left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonasdeluna commented Feb 26, 2024

jonasdeluna commented Feb 6, 2024 •

edited

jonasdeluna commented Feb 20, 2024 •

edited

norbye left a comment •

edited