lucid icon indicating copy to clipboard operation
lucid copied to clipboard
verified publisher icon adonisjs

feat: uniqueRaw and unique

Open Xstoudi opened this issue 1 year ago • 2 comments

🔗 Linked issue

#995

❓ Type of change

  • [ ] 🐞 Bug fix (a non-breaking change that fixes an issue)
  • [ ] 👌 Enhancement (improving an existing functionality like performance)
  • [x] ✨ New feature (a non-breaking change that adds functionality)
  • [ ] ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

Current unique rule becomes uniqueRaw as proposed by @thetutlage on #995 and we add a new unique allowing a simpler and more expressive API inspired from Adonis v5.

Feel free to comment the implem, I'm not fully familiar with Adonis codebase. I couldn't found the doc about future old unique rule, can you show me where it's mentionned in the doc so I can update the doc?

📝 Checklist

  • [x] I have linked an issue or discussion.
  • [ ] I have updated the documentation accordingly.

Xstoudi avatar Mar 11 '24 22:03 Xstoudi

Hey, both unique and uniqueRaw could be the same overload function. Because, right now this PR will lead to a breaking change, and that's something we should avoid.

Also let's add some tests for the new implementation.

Feel free to comment the implem, I'm not fully familiar with Adonis codebase. I couldn't found the doc about future old unique rule, can you show me where it's mentionned in the doc so I can update the doc?

Haha, seems like it never got documented. So we will have to document it from scratch.

thetutlage avatar Mar 12 '24 02:03 thetutlage

It use overload now.

I'm not sure how to get started to write test for a binding and for as much as I looked, I cannot use previous test to get inspiration because they do not exist (or I can't find it).

Where do you think we should document it? On the Lucid doc with a Rules contributed by Lucid like this:https://docs.adonisjs.com/guides/validation#rules-contributed-by-adonisjs ?

Xstoudi avatar Mar 12 '24 17:03 Xstoudi

Hello, will it be merged soon?

I have two questions:

  • Can we add a caseInsensitive option like in the previous Lucid version?
  • What do you think about adding the same Options parameter to the exists rule?

I can open another PR if you want.

flozdra avatar Nov 13 '24 19:11 flozdra

Sorry slept on the PR for quite long as did not have the energy to manage multiple things.

I will merge this PR and then do other improvements like adding caseInsensitive and bringing the same options to the exists validation rule

thetutlage avatar Dec 04 '24 06:12 thetutlage

Improved both the validations. https://github.com/adonisjs/lucid/commit/761f8231c56891c1f4682cf9a92637060e83346d

thetutlage avatar Dec 04 '24 11:12 thetutlage