-
Notifications
You must be signed in to change notification settings - Fork 309
Rich object string parameters for notifications #2935
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
Conversation
dfe0dd9 to
e3266ea
Compare
| ); | ||
| $notification->setRichSubject( | ||
| (string) $l->t('{user} has shared the board %s with you.', [$params[0]]), | ||
| $l->t('{user} has shared {deck-board} with you.'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure if we should keep "… the board …"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No strong opinions here, but I'd say the icon is already enough to indicate that it is deck related. Maybe @jancborchardt can comment about the general wording.
| $card = $this->cardMapper->find($cardId); | ||
| $stackId = $card->getStackId(); | ||
| $stack = $this->stackMapper->find($stackId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a shorter way for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not as of right now as we always use an entity and don't have any way to map joined results to it currently. I'd say we keep it like this and if we encounter more places where this would be useful we can add a helper to do this in one query.
Signed-off-by: Joas Schilling <[email protected]>
e3266ea to
e5d716c
Compare
|
Rebased and adjusted the tests |
|
Sorry, thanks for the tests fixing I forgot about the PR |
|
Sure thanks for the PR 👍 |
|
/backport to stable1.4 |
|
The backport to stable1.4 failed. Please do this backport manually. |
Fixes #2949