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

URL nullable? #84

Closed
eleftrik opened this issue Mar 1, 2016 · 6 comments
Closed

URL nullable? #84

eleftrik opened this issue Mar 1, 2016 · 6 comments
Assignees

Comments

@eleftrik
Copy link

eleftrik commented Mar 1, 2016

I think the URL field should be nullable.
I have some notifications which don't require the user to be redirected to an URL.
At the moment, I'm setting the URL to an empty string.
Thank you

@fenos
Copy link
Owner

fenos commented Mar 1, 2016

Thanks for your feedback, Notifynder try to be as generic and extendable as possible. For custom cases and custom scenarios Notifynder allow you to be personalised as needed.

For example if in your case you need the url as nullable then create a migration that make the field nullable.

The only current limitation is the builder which is not very extendable for your case and it require a url. I will keep this in mind for future releases thanks.

@eleftrik
Copy link
Author

eleftrik commented Mar 1, 2016

Yes, at the moment I think I'll create a migration to make that field nullable.
Thanks for your feedback.

@Gummibeer
Copy link
Collaborator

I solved this problem by using # as the url to prevent html using the current url. But nullable would be great and it won't break anything if the field is nullable in the package migrations.

@Gummibeer
Copy link
Collaborator

pls use the getRequiredFields() method in the exception message after merging the PR #106

Gummibeer added a commit that referenced this issue Apr 22, 2016
#84 make url nullable and don't require it
@Gummibeer
Copy link
Collaborator

PR #104 is merged - so this is done.

Gummibeer added a commit that referenced this issue Apr 22, 2016
pls use the getRequiredFields() method in the exception message after merging the PR #106
Gummibeer added a commit that referenced this issue Apr 22, 2016
@Gummibeer
Copy link
Collaborator

PR #108 is also merged so this issue is done and closed.

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

No branches or pull requests

3 participants