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 casting issues (issue: #2703) #2705

Merged
merged 13 commits into from
Jan 16, 2024
Merged

Fix casting issues (issue: #2703) #2705

merged 13 commits into from
Jan 16, 2024

Conversation

stubbo
Copy link
Contributor

@stubbo stubbo commented Jan 10, 2024

Summary

#2653 Added a cast inside of the setAttribute, intended to be used when accessing raw data from a model, not adding data.

This will cause some casts to break fully. If you have the encrypt cast it will try to decrypt a string before encrypting it which causes an exception.

I have explained this in more detail in issue #2703

Checklist

  • Add tests and ensure they pass
  • Add an entry to the CHANGELOG.md file
  • Update documentation for new features

@stubbo
Copy link
Contributor Author

stubbo commented Jan 10, 2024

I will update the changelog and see if there's any documentation to update later today

@stubbo stubbo changed the title Fix casting issues (#2703) Fix casting issues (issue: #2703) Jan 10, 2024
Copy link
Member

@GromNaN GromNaN left a comment

Choose a reason for hiding this comment

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

Hello @stubbo, thanks for investigating the issue and submitting a patch.

@hans-thomas you may be interested in reviewing this PR as you worked on this part previously.

tests/Casts/StringTest.php Outdated Show resolved Hide resolved
src/Eloquent/Model.php Outdated Show resolved Hide resolved
try {
$value = (string) BigDecimal::of((string) $value)->toScale((int) $decimals, RoundingMode::HALF_UP);

return new Decimal128($value);
Copy link
Member

Choose a reason for hiding this comment

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

👍 Good catch, the method asDecimal must return a string, as mandated in the phpdoc of the parent method.

src/Eloquent/Model.php Outdated Show resolved Hide resolved
stubbo and others added 2 commits January 10, 2024 13:43
Co-authored-by: Jérôme Tamarelle <jerome@tamarelle.net>
Co-authored-by: Jérôme Tamarelle <jerome@tamarelle.net>
Copy link

@niyazialpay niyazialpay left a comment

Choose a reason for hiding this comment

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

I tested this changes with my project and it seems solved the problem

@stubbo stubbo requested a review from GromNaN January 11, 2024 17:17
tests/QueueTest.php Outdated Show resolved Hide resolved
@stubbo
Copy link
Contributor Author

stubbo commented Jan 16, 2024

@GromNaN Do you want me to update the change log?
If so, what would you like to use for the version?

@GromNaN GromNaN merged commit a5cf5cb into mongodb:4.1 Jan 16, 2024
15 checks passed
@GromNaN
Copy link
Member

GromNaN commented Jan 16, 2024

Changelog updated in 749a2d0

Thank you @stubbo

@hans-thomas
Copy link
Contributor

Hi @GromNaN, I will be unavailable for next month too. sorry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants