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

fix: Post--by-start-user not working #3356

Merged
merged 1 commit into from
Mar 23, 2022
Merged

Conversation

SychO9
Copy link
Member

@SychO9 SychO9 commented Mar 20, 2022

Changes proposed in this pull request:
This is an odd one, #3170 was supposed to fix this, but I'm not sure where I got startUserId from, that has never been part of the discussion frontend attributes. (and the title of that fix refers to the wrong class as well).

This PR loads the author relationship by default to compare against.

Reviewers should focus on:
We could also just introduce the attribute above, instead of loading the relationship.

Necessity

  • Has the problem that is being solved here been clearly explained?
  • If applicable, have various options for solving this problem been considered?
  • For core PRs, does this need to be in core, or could it be in an extension?
  • Are we willing to maintain this for years / potentially forever?

Confirmed

  • Frontend changes: tested on a local Flarum installation.
  • Backend changes: tests are green (run composer test).
  • Core developer confirmed locally this works as intended.
  • Tests have been added, or are not appropriate here.

@luceos
Copy link
Member

luceos commented Mar 23, 2022

Does this PR need to have upstream changes for tests to succeed?

@SychO9
Copy link
Member Author

SychO9 commented Mar 23, 2022

it will be merged after #3348 is merged.

@askvortsov1
Copy link
Sponsor Member

Yep, although it will need to be updated since Post was converted to TypeScript in #3348

@SychO9 SychO9 merged commit 46f8cf4 into main Mar 23, 2022
@SychO9 SychO9 deleted the sm/fix-post-by-start-user branch March 23, 2022 17:21
@SychO9 SychO9 added this to the 1.3 milestone Mar 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants