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

Hackathon: Post Reminders #20555

Merged
merged 16 commits into from
Jul 26, 2022
Merged

Hackathon: Post Reminders #20555

merged 16 commits into from
Jul 26, 2022

Conversation

agnivade
Copy link
Member

@agnivade agnivade commented Jun 27, 2022

This PR adds the post reminder backend work.

We add a new API endpoint via which a user can set a reminder for a post. An ephemeral message will be sent down the line to let the user know about the action. And then after the time is over, the system admin bot will send a DM message to the user about the reminder post.

Added the server-side code for post reminders

agnivade added 7 commits June 27, 2022 11:04
```release-note
NONE
```
```release-note
NONE
```
```release-note
NONE
```
```release-note
NONE
```
```release-note
NONE
```
```release-note
NONE
```
```release-note
NONE
```
@agnivade agnivade added the 2: Dev Review Requires review by a developer label Jun 27, 2022
@mm-cloud-bot
Copy link

@agnivade: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

I understand the commands that are listed here

@agnivade agnivade changed the title postReminder Hackathon: Post Reminders Jun 27, 2022
```release-note
NONE
```
@mm-cloud-bot mm-cloud-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed labels Jun 28, 2022
```release-note
NONE
```
Copy link
Member

@isacikgoz isacikgoz left a comment

Choose a reason for hiding this comment

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

Looking forward to use this 🥇 did my first pass and have some questions.

api4/post_test.go Show resolved Hide resolved
app/post.go Show resolved Hide resolved
app/post.go Outdated Show resolved Hide resolved
@agnivade agnivade requested a review from isacikgoz July 5, 2022 05:20
```release-note
NONE
```
Copy link
Member

@isacikgoz isacikgoz left a comment

Choose a reason for hiding this comment

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

Noice 💯

@agnivade
Copy link
Member Author

agnivade commented Jul 5, 2022

Oh I need to add the api reference as well.

Copy link
Contributor

@ashishbhate ashishbhate left a comment

Choose a reason for hiding this comment

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

LGTM.

Two small comments inline. One non-blocking discussion, other comment just highlights a small typo.

i18n/en.json Outdated Show resolved Hide resolved
return
}

reminders, err := a.Srv().Store.Post().GetPostReminders(time.Now().UTC().Unix())
Copy link
Contributor

@ashishbhate ashishbhate Jul 7, 2022

Choose a reason for hiding this comment

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

In a.Srv().Store.Post().GetPostReminders() we delete reminders after getting them.

If there there are any failures in this app layer function after this point, then those reminders wont be sent and will be lost.

Maybe we ought to consider splitting up the get and delete in the store into separate operations?

Non-blocking comment for this PR. We can handle this later.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, I agree we can handle this later. I'll add a comment and create a ticket.

@agnivade agnivade added the Setup Cloud Test Server Setup an on-prem test server label Jul 7, 2022
@agnivade agnivade removed the Setup Cloud Test Server Setup an on-prem test server label Jul 8, 2022
@matthewbirtch matthewbirtch added the Setup Cloud Test Server Setup an on-prem test server label Jul 10, 2022
@agnivade agnivade added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a developer labels Jul 26, 2022
@mm-cloud-bot mm-cloud-bot added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Jul 26, 2022
@mm-cloud-bot mm-cloud-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Jul 26, 2022
@ashishbhate
Copy link
Contributor

The frontend component for this feature will take a lot more time to be ready. This PR is ready, and keeping it up to date is unnecessary work.
Merging this PR. Any changes required by the frontend can come in follow-up PRs.

@ashishbhate ashishbhate merged commit 20cb042 into master Jul 26, 2022
@ashishbhate ashishbhate deleted the postReminder branch July 26, 2022 10:42
@mattermod mattermod removed the Setup Cloud Test Server Setup an on-prem test server label Jul 26, 2022
@mm-cloud-bot
Copy link

Test server destroyed

isacikgoz added a commit that referenced this pull request Jul 28, 2022
* Revert "Removed the opengraph type dependency for plugins by stripping post Metadata (#20612)" (#20684)

This reverts commit da7a8a4.

* Update latest version to 7.1.2 (#20691)

Automatic Merge

* MM-45208: strip post meta for plugins (#20686)

Try two...

* Revert "Revert "Removed the opengraph type dependency for plugins by stripping post Metadata (#20612)" (#20684)"

This reverts commit 32dee6d.

* Fixed race condition

* PR feedback

* lint

* [MM-44948] Update sync (#20414)

* Update sync

* Add enter

* Address review suggestion

Co-authored-by: Mattermod <[email protected]>

* Store plugin OnActivate errors and include them in PluginStatus response (#20430)

* store plugin OnActivate errors, and include in PluginStatus

* write test

Co-authored-by: Mattermod <[email protected]>

* [MM-42194] Get file information from a deleted post (#20279)

* Introduced inlcude_deleted query parameter to allow admins to retrieve contents of post regardless of deletion status

* Introduced new client route and tests for getting file info of deleted posts

* Fixed tests due to caching of posts

* gofmt

* Small formatting updates

* Invalidating file infos cache on delete of post if post includes files

* Including deleted in migration flow

* Moved invalidating of cache

Co-authored-by: Mattermod <[email protected]>

* [MM-45862] Set size and extension to fileinfo (#20681)

* Set size and extension to fileinfo

* Remove unnecessary check

* added a feature-flag and default value for PostForwarding (#20166)

Automatic Merge

* MM-45000: Upgrade to new opengraph module path (#20701)


https://mattermost.atlassian.net/browse/MM-45000

```release-note
NONE
```

* MM-45875: Apply environment overrides in cluster scenario (#20694)

Automatic Merge

* move metrics server into platform service (#20683)

move metrics into platform

* Killing dyatlov/go-opengraph with fire (#20706)


Correctly updating this requires PRs
in multiple repositories. Fixing this
with a hammer for now to unblock server.

```release-note
NONE
```

* [MM-45666] - A/B Test: Nav Bar Upgrade button (#20676)

* [MM-45666] - A/B Test: Nav Bar Upgrade button

* update flag default value

* update text

* Mm 43609 (#20657)

* return first inaccessible post time instead of has inaccessible posts

* MM-45871: Do not try to extract content from images (#20698)

This creates faulty requests to Bifrost and results in
errors and warnings in the logs. Even without Bifrost,
this would make unnecessary requests to S3.

We only extract info from documents and therefore we can
safely avoid this.

```release-note
NONE
```

* Translated using Weblate (Turkish)

Currently translated at 100.0% (2339 of 2339 strings)

Translation: mattermost-languages-shipped/mattermost-server
Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-server_master/tr/

* Translated using Weblate (German)

Currently translated at 100.0% (2339 of 2339 strings)

Translation: mattermost-languages-shipped/mattermost-server
Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-server_master/de/

* Translated using Weblate (Korean)

Currently translated at 84.5% (1978 of 2339 strings)

Translation: mattermost-languages-shipped/mattermost-server
Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-server_master/ko/

Translated using Weblate (Korean)

Currently translated at 83.9% (1963 of 2339 strings)

Translation: mattermost-languages-shipped/mattermost-server
Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-server_master/ko/

* Added translation using Weblate (Vietnamese)

* Deleted translation using Weblate (Vietnamese)

* Translated using Weblate (Hungarian)

Currently translated at 99.5% (2328 of 2339 strings)

Translation: mattermost-languages-shipped/mattermost-server
Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-server_master/hu/

* Translated using Weblate (English (Australia))

Currently translated at 99.2% (2322 of 2339 strings)

Translation: mattermost-languages-shipped/mattermost-server
Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-server_master/en_AU/

* Translated using Weblate (French)

Currently translated at 100.0% (2339 of 2339 strings)

Translation: mattermost-languages-shipped/mattermost-server
Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-server_master/fr/

* Translated using Weblate (Japanese)

Currently translated at 100.0% (2339 of 2339 strings)

Translation: mattermost-languages-shipped/mattermost-server
Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-server_master/ja/

* Translated using Weblate (Polish)

Currently translated at 100.0% (2339 of 2339 strings)

Translation: mattermost-languages-shipped/mattermost-server
Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-server_master/pl/

* Hackathon: Post Reminders (#20555)

This PR adds the post reminder backend work.

We add a new API endpoint via which a user can set a reminder for a post. An ephemeral message will be sent down the line to let the user know about the action. And then after the time is over, the system admin bot will send a DM message to the user about the reminder post.

* [MM-45817] Set cloud cookies whenever user gets logged in (#20692)

* support plugin setting property `hosting` (#20677)

Co-authored-by: Mattermod <[email protected]>

* Added schema migration template to PR template (#20717)

A schema migration template is now available to help guide release note development.

* MM-45863: Move metrics under the Platform service

```release-note
NONE
```

* Fix initialization

```release-note
NONE
```

* Fix tests

```release-note
NONE
```

* fix merge defects

Co-authored-by: Mattermod <[email protected]>
Co-authored-by: Lev <[email protected]>
Co-authored-by: Shota Gvinepadze <[email protected]>
Co-authored-by: Michael Kochell <[email protected]>
Co-authored-by: Riccardo Santoni <[email protected]>
Co-authored-by: Claudio Costa <[email protected]>
Co-authored-by: Michel Engelen <[email protected]>
Co-authored-by: Ibrahim Serdar Acikgoz <[email protected]>
Co-authored-by: Allan Guwatudde <[email protected]>
Co-authored-by: Nathaniel Allred <[email protected]>
Co-authored-by: Kaya Zeren <[email protected]>
Co-authored-by: jprusch <[email protected]>
Co-authored-by: yeongeun.seo <[email protected]>
Co-authored-by: Weblate <[email protected]>
Co-authored-by: Tom De Moor <[email protected]>
Co-authored-by: Tóth Csaba // Online ERP Hungary Kft <[email protected]>
Co-authored-by: Matthew Williams <[email protected]>
Co-authored-by: Pierre JENICOT <[email protected]>
Co-authored-by: kaakaa <[email protected]>
Co-authored-by: master7 <[email protected]>
Co-authored-by: Julien Tant <[email protected]>
Co-authored-by: Carrie Warner (Mattermost) <[email protected]>
@amyblais amyblais added this to the v7.3.0 milestone Aug 1, 2022
@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Aug 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4: Reviews Complete All reviewers have approved the pull request Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation release-note Denotes a PR that will be considered when it comes time to generate release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants