However in the process I also discovered that the private discussions profile tab just didn't seem to work anymore since v1.0 of Byobu, because the gambit it uses was removed. I have re-added the gambit and used a method from the existing trait to not duplicate any logic.
Reviewers should focus on:
Do we still want the private discussions page on the profile so it it worth fixing it?
Do we need backward compatibility with how the byobu search gambit worked? I have dropped support for multiple usernames in a single query, and now it will no longer work with usernames if the user slugger is switched because it uses the slugger to convert the value
Should the forRecipient() trait method be made public since it's now used directly from elsewhere? (it's protected)
We could rename the route parameter from username to slug, but Flarum core also kept its profile route parameters named username so I think it's more logical to keep it as-it, and just pass the slug value to the parameter
I wasn't 100% sure what the page on the user profile was actually supposed to do. I guess it makes sense to list all private discussions we have with that particular user, without regard for whether they started the discussion or somebody else started it. But I'm not sure how it was done in 0.6, having never used the extension myself.
Confirmed
[x] Frontend changes: tested on a local Flarum installation.
[ ] Backend changes: tests are green (run composer test).
I think we do have the javascript build running on master? I purposefully not included the dist files in the PR, but I think it's all good on master now.
Changes proposed in this pull request: The main goal of this PR was to add support for custom user sluggers, in particular Flarum's built-in ID-based slugger, following this request https://discuss.flarum.org/d/4762-friendsofflarum-by-bu-well-integrated-advanced-private-discussions/1119
However in the process I also discovered that the private discussions profile tab just didn't seem to work anymore since v1.0 of Byobu, because the gambit it uses was removed. I have re-added the gambit and used a method from the existing trait to not duplicate any logic.
Reviewers should focus on:
byobu
search gambit worked? I have dropped support for multiple usernames in a single query, and now it will no longer work with usernames if the user slugger is switched because it uses the slugger to convert the valueforRecipient()
trait method be madepublic
since it's now used directly from elsewhere? (it'sprotected
)username
toslug
, but Flarum core also kept its profile route parameters namedusername
so I think it's more logical to keep it as-it, and just pass the slug value to the parameterI wasn't 100% sure what the page on the user profile was actually supposed to do. I guess it makes sense to list all private discussions we have with that particular user, without regard for whether they started the discussion or somebody else started it. But I'm not sure how it was done in 0.6, having never used the extension myself.
Confirmed
composer test
).