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

[MM-61166] Ensure full validation of Calls post props #885

Merged
merged 3 commits into from
Oct 24, 2024
Merged

Conversation

streamer45
Copy link
Collaborator

Summary

Since props cannot be trusted (i.e., users can alter them at any time on their posts), we must take extra care to ensure that the type of them is what we expect it to be.

Ticket Link

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

Copy link
Member

@cpoile cpoile left a comment

Choose a reason for hiding this comment

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

Nice!

Comment on lines 587 to 588
recordings: isValidObject(post.props?.recordings) ? post.props.recordings : {},
transcriptions: isValidObject(post.props?.transcriptions) ? post.props.transcriptions : {},
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to validate that the recordings and transcriptions objects are valid?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@cpoile They are not used right now, but we should. Let me tackle that as part of https://mattermost.atlassian.net/browse/MM-61228

@streamer45 streamer45 added 3: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core committer labels Oct 24, 2024
@streamer45 streamer45 added this to the v1.3.0 / MM v10.3 milestone Oct 24, 2024
@streamer45 streamer45 merged commit 74d6f31 into main Oct 24, 2024
19 checks passed
@streamer45 streamer45 deleted the MM-61166 branch October 24, 2024 22:10
streamer45 added a commit that referenced this pull request Oct 25, 2024
* Ensure full validation of Calls post props

* [MM-61228] Surface transcriptions in call post (#886)

* Surface transcriptions in call post

* Add proper job metadata validation
streamer45 added a commit that referenced this pull request Oct 25, 2024
* Ensure full validation of Calls post props

* [MM-61228] Surface transcriptions in call post (#886)

* Surface transcriptions in call post

* Add proper job metadata validation
streamer45 added a commit that referenced this pull request Oct 25, 2024
* [MM-61166] Ensure full validation of Calls post props (#885)

* Ensure full validation of Calls post props

* [MM-61228] Surface transcriptions in call post (#886)

* Surface transcriptions in call post

* Add proper job metadata validation

* [MM-61298] Fix e2e pipeline (#888)

* Fix e2e pipeline

* We don't need ES

* Fix docker login step (#881)

* Fix docker login step

* Update .github/workflows/e2e.yml

Co-authored-by: Mario Vitale <mvitale1989@hotmail.com>

---------

Co-authored-by: Mario Vitale <mvitale1989@hotmail.com>

* Fix e2e pipeline flakyness (#867)

---------

Co-authored-by: Mario Vitale <mvitale1989@hotmail.com>
streamer45 added a commit that referenced this pull request Oct 25, 2024
* [MM-61298] Fix e2e pipeline (#888)

* Fix e2e pipeline

* We don't need ES

* Fix docker login step (#881)

* Fix docker login step

* Update .github/workflows/e2e.yml

Co-authored-by: Mario Vitale <mvitale1989@hotmail.com>

---------

Co-authored-by: Mario Vitale <mvitale1989@hotmail.com>

* [MM-61166] Ensure full validation of Calls post props (#885)

* Ensure full validation of Calls post props

* [MM-61228] Surface transcriptions in call post (#886)

* Surface transcriptions in call post

* Add proper job metadata validation

---------

Co-authored-by: Mario Vitale <mvitale1989@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3: Reviews Complete All reviewers have approved the pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants