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

Form_validation: remove empty rules in products & tasks models same other models #1195

Draft
wants to merge 10 commits into
base: next-1.7
Choose a base branch
from

Conversation

sudwebdesign
Copy link
Contributor

@sudwebdesign sudwebdesign commented Jan 14, 2025

Description

Same as numerous Models (Good practice?)
See modules/clients/models/Mdl_clients.php line 40
in validation_rules: client_title, client_surname, ...
have no rule

Same in modules/invoices/models/Mdl_items.php line 66 and others...

Related Issue

Maybe #1180

Motivation and Context

Completion to simplify transition for php.8.4 & maintained CI
Compatibility with CodeIgniter 3.1.13 & 3.3.0 by @pocketarc
In relation with PR #1161 + Issue #1180

Exception find when use CodeIgniter 3.3.0 by @pocketarc
and when edit + save product go to blank page (prod mode)
See this message for backtrace.

Pull Request Checklist

  • My code follows the code formatting guidelines.
  • I have an issue ID for this pull request.
  • I selected the corresponding branch.
  • I have rebased my changes on top of the corresponding branch.

Issue Type (Please check one or more)

  • Bugfix (with CodeIgniter 3.3.0 by @pocketarc)
  • Improvement of an existing Feature
  • New Feature

AeroBytesNL and others added 10 commits December 31, 2024 16:25
Now Custom Fields ip_invoice_custom is present in settings page

+ Remove hard fix in view template-tags-invoices

+ Indents of settings view partial_settings general
Inspired by `module/clients/view/partial_client_table.php`
Improve InvoicePlane#1185

Scope:
+ Clients
+ DashBoard
+ Invoices
+ Quotes
+ Payments
+ Products
+ Tasks

Note: .amount.last apply padding in last element like Quotes list
`$(document).on('click', '.ajax-loader', function () {`
Is duplicated inside same function.
@sudwebdesign
Copy link
Contributor Author

CI (3.3.0) say after save product (Message in dev mode)

An uncaught Exception was encountered

Type: RuntimeException

Message: Form_validation: set_rules() called with an empty $rules parameter

Filename: vendor/pocketarc/codeigniter/system/libraries/Form_validation.php

Line Number: 202

Backtrace:

File: application/core/MY_Model.php
Line: 456
Function: set_rules

File: application/modules/products/controllers/Products.php
Line: 53
Function: run_validation

File: index.php
Line: 329
Function: require_once

Compare CI's to see why
Search Form_validation.php in tab files & Load Diff

@nielsdrost7 nielsdrost7 changed the base branch from development to next-1.7 January 14, 2025 16:40
@nielsdrost7 nielsdrost7 marked this pull request as draft January 14, 2025 16:40
@nielsdrost7
Copy link
Contributor

@sudwebdesign Thank you for the PR.
Wasn't it true, that all validation rules needed a rule?
Last time I tested PHP 8.3 (in that case) + a different fork of CI, I ran into the problem that they all needed a rule.
I like the fact that the empty rules need to be deleted and that the problem then is resolved.

I've done 2 things:

  • Set the PR to draft (just to be sure)
  • set the base to "next-1.7" which could be the version 1.7 that belongs to CI by pocketarc and compatibility with PHP 8.2+

@nielsdrost7
Copy link
Contributor

I need to keep development for the "1.6.3" version.
I can still merge your solution in version 1.6.3, no problem at all, I'm just keeping them separate for the moment

@sudwebdesign
Copy link
Contributor Author

sudwebdesign commented Jan 14, 2025

@nielsdrost7

set the base to "next-1.7" which could be the version 1.7 that belongs to CI by pocketarc and compatibility with PHP 8.2+

Just in case: this change no affect (maybe accelerate) the product/task save with CI3.1.13 (It's ok for IP v1.6.3, i checked)

Wasn't it true, that all validation rules needed a rule?

IMO On Original CI 3 when if rule is set to '' return $this and execution more longest. (see)*
It's good, if field no need validation rule, don't need rule in set (more logically).

Last time I tested PHP 8.3 (in that case) + a different fork of CI, I ran into the problem that they all needed a rule.

Thanks for your big work and this report of other forks.

I like the fact that the empty rules need to be deleted and that the problem then is resolved.

Same thing (KISS principle) 💯

Note:

Theses Exceptions is created in 2018 by @narfbg (a long maintainer of CI?) (+ this) not the fact of @pocketarc
See : https://github.com/bcit-ci/CodeIgniter/blob/develop/system/libraries/Form_validation.php#L202

*With CodeIgniter-3.1.13 release do not have this but CI by @pocketarc these errors are being reported.
This is the why of this PR ;)

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

Successfully merging this pull request may close these issues.

5 participants