Skip to content

fix: accept nil values when validating URLs#150

Open
iloveitaly wants to merge 1 commit intomainfrom
accept-nil
Open

fix: accept nil values when validating URLs#150
iloveitaly wants to merge 1 commit intomainfrom
accept-nil

Conversation

@iloveitaly
Copy link
Collaborator

without this, it's impossible to allow nil values.

nil constraints should be set elsewhere and be allowed in this library from my pov.

@codecov
Copy link

codecov bot commented Aug 21, 2023

Codecov Report

Merging #150 (68995a5) into main (8c57df9) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main      #150   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           23        23           
  Lines          189       190    +1     
=========================================
+ Hits           189       190    +1     
Files Changed Coverage Δ
lib/url.ex 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@nelsonic
Copy link
Member

Hi @iloveitaly 👋
Could you please clarify the use case for a nil value being OK? 💭

@iloveitaly
Copy link
Collaborator Author

Not all fields on my model are required. In this case, a nil value could be passed and converted into a NULL on the DB side. Essentially, I want to leave a field blank.

@nelsonic
Copy link
Member

Hmm ... this feels like something that can be checked in the changeset/2 function
or defined declaratively in the schema ...
I'm not against merging/publishing this change necessarily,
I'm just curious if this can be achieved without allowing nil (NULL) values for URLs because nil isn't a valid URL ... 💭

@iloveitaly
Copy link
Collaborator Author

@nelsonic do you know what I need to add to:

schema "things" do
  field :url, Fields.Url
end

so a null/nil value is accepteed?

@iloveitaly
Copy link
Collaborator Author

@nelsonic friendly ping here!

@nelsonic nelsonic self-assigned this Sep 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: More ToDo ThanCanEver Be Done
Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants