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

Fix: populate param parsing #394

Merged
merged 5 commits into from
Feb 4, 2025
Merged

Fix: populate param parsing #394

merged 5 commits into from
Feb 4, 2025

Conversation

Freezystem
Copy link
Contributor

I just noticed that params used in the getNativeVirtualPopulateQuery weren't the sanitized ones.
So I've added a parameter to pass the sanitized params along with the context.

One question remains though:

Why are we not directly altering ctx.params from the context instead of passing a sanitized value along with the context in methods ? It would avoid passing an additional parameter and prevent mistaking original params for sanitized ones.

@Freezystem Freezystem force-pushed the fix/populate-param-parsing branch from 03b7f10 to 666a35d Compare August 27, 2024 15:30
@Freezystem
Copy link
Contributor Author

@icebob Can I have your review and opinion on this one ? 🙇🏼

icebob
icebob requested changes Feb 3, 2025
View reviewed changes
Copy link
Member

@icebob icebob left a comment

Choose a reason for hiding this comment

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

Plz revert the changes of package-lock file.

@Freezystem Freezystem force-pushed the fix/populate-param-parsing branch from 15c431d to 6759f29 Compare February 4, 2025 08:33
@Freezystem Freezystem force-pushed the fix/populate-param-parsing branch from 6759f29 to 0a3689f Compare February 4, 2025 08:50
@Freezystem
Copy link
Contributor Author

Actually my previous PR has migrated the lockfile version to v2.
I've reverted this change and rollbacked it to v1.
Let me know if it's ok.

@icebob
Copy link
Member

icebob commented Feb 4, 2025

Thanks, all good

@icebob icebob merged commit e1cf694 into moleculerjs:master Feb 4, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@icebob icebob icebob requested changes

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

Successfully merging this pull request may close these issues.
2 participants
@Freezystem @icebob