Skip to content

Conversation

@islamaliev
Copy link
Contributor

Relevant issue(s)

Resolves #3146

Description

Fix default json value by properly converting it's value into a corresponding type.

@islamaliev islamaliev added this to the DefraDB v1.0 milestone Oct 9, 2025
@islamaliev islamaliev requested a review from a team October 9, 2025 21:45
@islamaliev islamaliev self-assigned this Oct 9, 2025
@islamaliev islamaliev added the bug Something isn't working label Oct 9, 2025
@codecov
Copy link

codecov bot commented Oct 9, 2025

Codecov Report

❌ Patch coverage is 71.42857% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.04%. Comparing base (c93310e) to head (68be936).
⚠️ Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
internal/request/graphql/schema/collection.go 71.43% 2 Missing and 2 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #4064      +/-   ##
===========================================
+ Coverage    73.97%   74.04%   +0.07%     
===========================================
  Files          477      477              
  Lines        43754    43766      +12     
===========================================
+ Hits         32367    32406      +39     
+ Misses        9176     9158      -18     
+ Partials      2211     2202       -9     
Flag Coverage Δ
all-tests 74.04% <71.43%> (+0.07%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
internal/request/graphql/schema/collection.go 82.43% <71.43%> (-0.15%) ⬇️

... and 9 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Collaborator

@ChrisBQu ChrisBQu left a comment

Choose a reason for hiding this comment

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

LGTM.

@islamaliev islamaliev merged commit da9e14b into sourcenetwork:develop Oct 10, 2025
115 of 125 checks passed
// Since setting a default value to nil is the same as not providing one,
// it is safer to return an error to let the user know something is wrong.
if value == nil {
// Exception: JSON fields can have nil as a valid default value
Copy link
Collaborator

Choose a reason for hiding this comment

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

question: Why do we treat the JSON type differently?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JSON object default value causes panic

3 participants