Skip to content
This repository was archived by the owner on Aug 21, 2024. It is now read-only.

Conversation

barankyle
Copy link
Contributor

@barankyle barankyle commented Nov 21, 2023

Summary

Updated code to support header population on internal calls.

Internal listeners that send messages to interested clients need
to persist headers on calls, as original headers are not preserved.

Fixed issues with email/SMS generation.

Made some resolver get's internal _get's, in cases where headers
may not be available at that point. All instances are cases where
headers might actively interfere with the process, e.g. looking
up static resources that might not have a field populated from
headers.

Converetd uploadMediaStaticResource and uploadRecordingStaticResource
into a single new service. uploadRecordingStaticResource technically
still exists, but it just uses its internal reference to the API
to call the new service. The code in both places was nearly identical,
and having it as a single service allows for header usage from outside
projects.

References

closes #insert number here

QA Steps

@barankyle barankyle marked this pull request as ready for review November 27, 2023 22:52
Copy link
Member

@hanzlamateen hanzlamateen left a comment

Choose a reason for hiding this comment

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

I have gone through the code and it looks good. Though havn't got a chance to test the changes locally.

@hanzlamateen
Copy link
Member

@barankyle used this branch of EE, standalone. Without any project, etc. Just default-project. Created a recording on capture page and the getting various errors on recordings page in admin panel.

Recording.2023-11-29.141713.mp4

@barankyle barankyle force-pushed the storage-domain branch 2 times, most recently from 03e148d to 0e48f12 Compare December 1, 2023 01:55
Copy link
Contributor

@HexaField HexaField left a comment

Choose a reason for hiding this comment

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

I don't like putting headers in the actions. It's a security risk and can be done cleaner.

@hanzlamateen
Copy link
Member

Do we need these changes after #9371

Copy link
Member

@hanzlamateen hanzlamateen left a comment

Choose a reason for hiding this comment

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

check-errors seems to be failing.

@barankyle barankyle force-pushed the storage-domain branch 2 times, most recently from e5532bd to 10c0aa5 Compare December 6, 2023 23:29
@barankyle barankyle dismissed HexaField’s stale review December 8, 2023 18:14

Implemented requested changes

Internal listeners that send messages to interested clients need
to persist headers on calls, as original headers are not preserved.

Fixed issues with email/SMS generation.

Made some resolver get's internal _get's, in cases where headers
may not be available at that point. All instances are cases where
headers might actively interfere with the process, e.g. looking
up static resources that might not have a field populated from
headers.

Converetd uploadMediaStaticResource and uploadRecordingStaticResource
into a single new service. uploadRecordingStaticResource technically
still exists, but it just uses its internal reference to the API
to call the new service. The code in both places was nearly identical,
and having it as a single service allows for header usage from outside
projects.
@speigg speigg added this pull request to the merge queue Dec 11, 2023
Merged via the queue into dev with commit b9ed746 Dec 11, 2023
@speigg speigg deleted the storage-domain branch December 11, 2023 20:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants