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.

Sign up for GitHub

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

Jump to bottom

Pass metadata from processPublishCommand to isPublishAllowed #4969

Merged
merged 7 commits into from Jun 17, 2023
Merged

Pass metadata from processPublishCommand to isPublishAllowed #4969

merged 7 commits into from Jun 17, 2023

Conversation

lastpeony
Copy link
Contributor

@lastpeony lastpeony commented Apr 3, 2023 •
edited by mekya

processPublishCommand function on enterprise takes metaData but it doesnt pass it to isPublishAllowed.

In previous PR metadata was retrieved through broadcast object to pass it webhook. In some cases(especially web conf) broadcast object is not created(null) thus eventhough client sends metadata it was not retrieved.

This is a refactor for this issue #4829

@lastpeony lastpeony force-pushed the metaDataOnAuthWebhookFix branch 2 times, most recently from a3d02b8 to 4f7325e Compare April 3, 2023 22:13
@mekya
Copy link
Contributor

mekya commented Apr 10, 2023

Hi @lastpeony ,
PR is failing. Please make it pass the CI and SonarCloud

@lastpeony lastpeony force-pushed the metaDataOnAuthWebhookFix branch 2 times, most recently from 4d3296e to 6c8e6d1 Compare April 12, 2023 09:29
@sonarcloud
Copy link

sonarcloud bot commented Apr 12, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

66.7% 66.7% Coverage
0.0% 0.0% Duplication

@lastpeony
Copy link
Contributor Author

Hi @lastpeony , PR is failing. Please make it pass the CI and SonarCloud

SonarCloud is reporting a low coverage error for a file that may not be related to the code changes. Can you please review and confirm if the file in question is indeed not related?

@mekya mekya mentioned this pull request Jun 12, 2023
Closed
8 tasks
@mekya
Copy link
Contributor

mekya commented Jun 17, 2023

This is a refactor for this issue #4829

@mekya mekya changed the title pass metadata from processPublishCommand to isPublishAllowed Pass metadata from processPublishCommand to isPublishAllowed Jun 17, 2023
mekya
mekya approved these changes Jun 17, 2023
View reviewed changes
Copy link
Contributor

@mekya mekya left a comment

Choose a reason for hiding this comment

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

Thank you for your work @lastpeony.

Just a reminder, changing interfaces has consequences so it should be changed everywhere. Providing backward compatibility is important. If you had provided it, there would be fewer changes in the codebase.

If we expose these APIs to the app developers, providing backward compatibility will be much more important than ever.

Thank you

@sonarcloud
Copy link

sonarcloud bot commented Jun 17, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

88.9% 88.9% Coverage
0.0% 0.0% Duplication

@mekya mekya merged commit 8609722 into master Jun 17, 2023
4 checks passed
@mekya mekya deleted the metaDataOnAuthWebhookFix branch June 17, 2023 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mekya mekya mekya approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@lastpeony @mekya