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

fix: simplify meteor collection auth checks #1381

Open
wants to merge 2 commits into
base: release53
Choose a base branch
from

Conversation

Julusian
Copy link
Contributor

@Julusian Julusian commented Feb 5, 2025

About the Contributor

This pull request is posted on behalf of the BBC

Type of Contribution

This is a: Bug fix / Code improvement

Current Behavior

The implementation of these auth checks is a confusing hack currently, due to bugs in meteor.

New Behavior

This updates meteor to the fixed version, and tidies up the hack

This removes some deprecation warnings
See meteor/meteor#13444

The vite config change is to resolve issues during development connecting to vite through a reverse proxy (which is important for testing the auth), as it would reject any hostname

Testing

  • I have added one or more unit tests for this PR
  • I have updated the relevant unit tests
  • No unit test changes are needed for this PR

Affected areas

Time Frame

Not urgent, but we would like to get this merged into the in-development release.

The version in r52 works, I see no reason to change it there.

Other Information

Status

  • PR is ready to be reviewed.
  • The functionality has been tested by the author.
  • Relevant unit tests has been added / updated.
  • Relevant documentation (code comments, system documentation) has been added / updated.

This removes some deprecation warnings
See meteor/meteor#13444
@Julusian Julusian requested a review from a team as a code owner February 5, 2025 12:16
Copy link

codecov bot commented Feb 5, 2025

Codecov Report

Attention: Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.

Please upload report for BASE (release53@55ffea8). Learn more about missing BASE report.

Files with missing lines Patch % Lines
meteor/server/collections/collection.ts 75.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             release53    #1381   +/-   ##
============================================
  Coverage             ?   56.25%           
============================================
  Files                ?      408           
  Lines                ?    73091           
  Branches             ?     4194           
============================================
  Hits                 ?    41115           
  Misses               ?    31735           
  Partials             ?      241           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jstarpl jstarpl added Contribution External contribution Contribution from BBC Contributions sponsored by BBC (bbc.co.uk) labels Feb 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Contribution from BBC Contributions sponsored by BBC (bbc.co.uk) Contribution External contribution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants