Skip to content

feat: implement permission change for selecting own post#124

Closed
BHZoon wants to merge 12 commits intoFriendsOfFlarum:1.xfrom
glowingblue:bh/select-own-best-answer-by-permission
Closed

feat: implement permission change for selecting own post#124
BHZoon wants to merge 12 commits intoFriendsOfFlarum:1.xfrom
glowingblue:bh/select-own-best-answer-by-permission

Conversation

@BHZoon
Copy link
Copy Markdown

@BHZoon BHZoon commented Dec 3, 2025

Fixes #0000

Changes proposed in this pull request:
Changing the way, the ability to select an own post as a best answer, is handled.
It was based on a setting and set for all users.
Change implementation to a permission check.
Also moved attribute away from the Discussion and into the Post

Confirmed

  • Frontend changes: tested on a local Flarum installation.
  • Backend changes: tests are green (run composer test).

Required changes:

@BHZoon BHZoon requested a review from a team as a code owner December 3, 2025 08:25
Copy link
Copy Markdown
Member

@DavideIadeluca DavideIadeluca left a comment

Choose a reason for hiding this comment

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

Appears like some changes in frontend logic are required to prevent showing the Select as Best Answer controls when not permitted to do so (in the following screencap the permission was set to admins only)

Screen.Recording.2025-12-03.at.14.38.36.mov

Besides that, there are some CI issues which need to be resolved in the frontend and backend. StyleCI doesn't automatically apply patches in Forks so you need to manually patch up the Code Style. The frontend contains both also some formatting issues (which can be fixed with yarn format) and also a typing error.

Besides all of this, this PR generally works and achieves the desired functionality. 👍

What I'm generally a little bit concerned is backwards compatibility: technically some changes warrant a MAJOR release as it contains breaking changes on paper (removal of DiscussionAttributes) class, removal of attributes in the frontend/backend). Practically, I'm not aware of anybody who would be affected by those changes here.. Maybe @imorland has opinions on this

Comment thread js/dist/admin.js
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's discouraged to commit dist files, this is automatically taken care of by CI. See https://docs.flarum.org/contributing/#setting-up-a-local-codebase

Comment thread js/package.json Outdated
Comment on lines +26 to +28
},
"dependencies": {
"yarn": "^1.22.22"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is unexpected and shouldn't be added to the package.json

Comment thread js/package-lock.json
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

the package-lock.json is unnecessary as we use yarn's yarn.lock for version locking

@DavideIadeluca
Copy link
Copy Markdown
Member

Closing this in favor of #127

@DavideIadeluca DavideIadeluca deleted the bh/select-own-best-answer-by-permission branch February 12, 2026 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants