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

feat: [CLI] Insert DBGroup variable in CLI validate method #8762

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from

Conversation

daycry
Copy link
Contributor

@daycry daycry commented Apr 11, 2024

Description
Allow a different DBGroup to be added in CLI class validations.

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@daycry daycry changed the title FIX: Insert DBGroup variable in CLI validate method fix: [CLI] Insert DBGroup variable in CLI validate method Apr 11, 2024
@kenjis
Copy link
Member

kenjis commented Apr 11, 2024

Please sign all commits.

We ask that contributions have code commits signed. This is important in order to prove, as best we can, the provenance of contributions.
See https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/signing.md

@kenjis kenjis added the tests needed Pull requests that need tests label Apr 11, 2024
@kenjis
Copy link
Member

kenjis commented Apr 11, 2024

Add test code.

We expect all code changes or bug-fixes to be accompanied by one or more tests added to our test suite to prove the code works.
https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/pull_request.md#contributions

@kenjis
Copy link
Member

kenjis commented Apr 11, 2024

@codeigniter4/core-team Is this a bug fix?

@kenjis kenjis added the GPG-Signing needed Pull requests that need GPG-Signing label Apr 11, 2024
@paulbalandan
Copy link
Member

This is a feature.

But I'm confused why CLI has to know the database group for prompts. It feels out of place here.

@kenjis kenjis added the wrong branch PRs sent to wrong branch label Apr 11, 2024
@daycry daycry changed the title fix: [CLI] Insert DBGroup variable in CLI validate method feature: [CLI] Insert DBGroup variable in CLI validate method Apr 11, 2024
@daycry daycry changed the title feature: [CLI] Insert DBGroup variable in CLI validate method feat: [CLI] Insert DBGroup variable in CLI validate method Apr 11, 2024
@kenjis kenjis added the breaking change Pull requests that may break existing functionalities label Apr 11, 2024
@daycry
Copy link
Contributor Author

daycry commented Apr 11, 2024

I I'm doing the secure commit but it doesn't detect it

@kenjis
Copy link
Member

kenjis commented Apr 11, 2024

@daycry What do you mean? What did you do exactly?

@kenjis
Copy link
Member

kenjis commented Apr 11, 2024

@daycry

The email in this signature doesn’t match the committer email.

@kenjis
Copy link
Member

kenjis commented Apr 11, 2024

@daycry Why do you need to add this?

@daycry
Copy link
Contributor Author

daycry commented Apr 11, 2024 via email

@kenjis
Copy link
Member

kenjis commented Apr 11, 2024

@daycry Okay, but this is an enhancement. So please change the base branch to 4.6.
See https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/workflow.md#if-you-sent-to-the-wrong-branch

@kenjis
Copy link
Member

kenjis commented Apr 11, 2024

@paulbalandan The database group is a part of the validation rules.
But it cannot be set in the rule. After all, there seems no good other way to set it.

I have a feeling that it would be better to set up the following:
is_unique[table.field,ignore_field,ignore_value,db_group]
However, if we do so, we will not be able to pass a DB connection instance.

@daycry
Copy link
Contributor Author

daycry commented Apr 11, 2024

I wan't pass DB connection instance, only DBGroup name, but I will try this.

is_unique[table.field,ignore_field,ignore_value,db_group]

Thanks

@kenjis kenjis added the enhancement PRs that improve existing functionalities label Apr 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Pull requests that may break existing functionalities enhancement PRs that improve existing functionalities GPG-Signing needed Pull requests that need GPG-Signing tests needed Pull requests that need tests wrong branch PRs sent to wrong branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants