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

Insert logic to validate empty string where the default is not null #9981

Merged
merged 6 commits into from Jan 27, 2020

Conversation

blutarch
Copy link
Member

@blutarch blutarch commented Jan 23, 2020

Please make sure these points are completed:

Along with vanilla/internal#2197, this will close issue vanilla/support#1239

The updated comboBreaker data was not being saved to the db.
Because the badge status was an empty string and didn't have a default value of null, it was not passing validation. This fix adds logic to validateRequired() in functions.validation that allows it to pass validation.

To Test:

  • Open the GDN_UserBadge table
  • Give a user a badge
  • Verify that the Attributes field for the comboBreaker badge (BadgeID = 3) updates with a new event
  • Give that user 4 more badges
  • Verify that the status field for the comboBreaker badge switches from NULL to given

@blutarch blutarch requested a review from a team January 23, 2020 22:15
@codecov
Copy link

codecov bot commented Jan 23, 2020

Codecov Report

Merging #9981 into master will decrease coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #9981      +/-   ##
============================================
- Coverage     52.29%   52.27%   -0.03%     
- Complexity     3766     7806    +4040     
============================================
  Files           362      362              
  Lines         43849    43818      -31     
============================================
- Hits          22931    22905      -26     
+ Misses        20918    20913       -5
Impacted Files Coverage Δ Complexity Δ
library/core/functions.validation.php 69.68% <100%> (+0.49%) 0 <0> (ø) ⬇️
...oard/controllers/api/AuthenticateApiController.php 83.27% <0%> (-0.27%) 62% <0%> (ø)
library/Vanilla/Models/InstallModel.php 58.53% <0%> (-0.26%) 51% <0%> (+51%)
...ry/Vanilla/EmbeddedContent/LegacyEmbedReplacer.php 70.73% <0%> (-0.24%) 0% <0%> (ø)
library/Vanilla/Authenticator/SSOAuthenticator.php 82.73% <0%> (-0.13%) 48% <0%> (+48%)
library/Vanilla/Models/FsThemeProvider.php 80.81% <0%> (-0.12%) 0% <0%> (ø)
...rd/controllers/api/AuthenticatorsApiController.php 89.47% <0%> (-0.1%) 28% <0%> (ø)
...s/dashboard/controllers/api/MediaApiController.php 85.02% <0%> (-0.08%) 39% <0%> (ø)
...ns/vanilla/controllers/api/DraftsApiController.php 89.75% <0%> (-0.07%) 43% <0%> (ø)
...ons/controllers/api/ConversationsApiController.php 92.4% <0%> (-0.07%) 57% <0%> (ø)
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d1c33fe...1790ceb. Read the comment docs.

@blutarch blutarch changed the title Insert logic to validate comboBreaker with status value of empty string Insert logic to validate empty string where the default is not null Jan 24, 2020
Copy link
Contributor

@tburry tburry left a comment

Choose a reason for hiding this comment

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

This is 99% done. Two small issues.

tests/Library/Core/ValidateRequiredTest.php Outdated Show resolved Hide resolved
library/core/functions.validation.php Show resolved Hide resolved
@blutarch blutarch requested a review from tburry January 27, 2020 13:26
@danni-stark
Copy link
Contributor

Tested
UserBadge.Attributes updating with the events
Screen Shot 2020-01-27 at 1 17 27 PM
Screen Shot 2020-01-27 at 1 18 56 PM

@tburry tburry merged commit e7104f4 into master Jan 27, 2020
@charrondev charrondev deleted the fix/combo-breaker-validation branch February 4, 2022 20:57
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.

None yet

3 participants