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

SC_CheckError 周りを諸々改修 #1157

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

SC_CheckError 周りを諸々改修 #1157

wants to merge 10 commits into from

Conversation

seasoftjapan
Copy link
Contributor

fixed #1115
fixed #1150
fixed #1156

Copy link

codecov bot commented Jan 21, 2025

Codecov Report

Attention: Patch coverage is 96.42857% with 3 lines in your changes missing coverage. Please review.

Project coverage is 49.95%. Comparing base (55648f6) to head (04af52a).

Files with missing lines Patch % Lines
data/class/SC_CheckError.php 98.75% 1 Missing ⚠️
data/class/helper/SC_Helper_Customer.php 0.00% 1 Missing ⚠️
data/class/helper/SC_Helper_HandleError.php 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1157      +/-   ##
==========================================
- Coverage   50.05%   49.95%   -0.11%     
==========================================
  Files          82       82              
  Lines       10580    10560      -20     
==========================================
- Hits         5296     5275      -21     
- Misses       5284     5285       +1     
Flag Coverage Δ
tests 49.95% <96.42%> (-0.11%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

EXIST_CHECK_REVERSE を使う必要が無くなり、EXIST_CHECK を使う。

fixed #1150
PHPUnit の Incomplete 2件 減少する。
@nanasess
Copy link
Contributor

@seasoftjapan PHP8.1以降の PostgreSQL でユニットテストが落ちているようですのでご確認お願いいたします🙇‍♂️

@seasoftjapan
Copy link
Contributor Author

seasoftjapan commented Jan 21, 2025

@nanasess
PostgreSQL だと PREF_CHECK で "\a" が渡ると落ちるようですね。
しかも、SC_Query を force で使っているので、落ちたことを捕捉できず OK で通ってしまい、後続の DB 処理が落ちると。。。

ff82ddc で、E_USER_ERROR を捕捉できるようになったはずなので、force 使わない形の着地を目指します。

その上で、"\a" はちょっと考えます。現況の SC_Helper_Customer::sfCustomerCommonParam() は、多分実際にDBエラーを発生させられそうですね。PREF_CHECK の前に NUM_CHECK を指定しても回避できそうですが、後々分かりにくいと思うので、PREF_CHECK の中で NUM_CHECK 相当のチェックも行う方向で考えています。→ そもそも getMasterData() すれば、DB 処理が不要な気がしてきました。

seasoftjapan added a commit that referenced this pull request Jan 21, 2025
都度のDB問い合わせを避ける。
PostgreSQL で発生していた制御文字のDBエラー回避を兼ねる。#1157 (comment)

SC_Helper_Customer::sfCustomerCommonParam() PREF_CHECK を後回しにした。前述の変更により、必然性は低下したが、妥当と考える。
ファイルパスをログ出力しようかと思ったが、この規則は本体で利用されていない様子なので一旦オミット
ff82ddc に伴い可能となった。
都度のDB問い合わせを避ける。
PostgreSQL で発生していた制御文字のDBエラー回避を兼ねる。#1157 (comment)

SC_Helper_Customer::sfCustomerCommonParam() PREF_CHECK を後回しにした。前述の変更により、必然性は低下したが、妥当と考える。
@seasoftjapan
Copy link
Contributor Author

@nanasess
SC_DB_MasterData::getMasterData('mtb_pref') を使い解決しました。04af52a
SC_Query の force も使わないよう変更しました。eee5772

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