Skip to content

Conversation

@BHZoon
Copy link

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

Copy link
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

js/package.json Outdated
Comment on lines 26 to 28
},
"dependencies": {
"yarn": "^1.22.22"
Copy link
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

Copy link
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

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