-
Notifications
You must be signed in to change notification settings - Fork 5
Fix custom fields conflict with standard columns #11
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
Fix custom fields conflict with standard columns #11
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r+ with comments, thanks!!
src/Concerns/Customizable.php
Outdated
@@ -46,13 +46,11 @@ public function validateCustomFields(): void | |||
if ($customFields->isEmpty()) { | |||
return; | |||
} | |||
$modelAttributeKeys = $modelAttributes->keys(); | |||
$customFieldColumns = $modelAttributeKeys->diff($tableColumns); | |||
$externalCustomFields = $modelAttributes->get('custom'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
可能要考慮到custom key拿到的值是null的情況,下面的validator在呼叫make的時候可能只接受array
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💬 comment
好奇為什麼這個變數要加上external
的前綴?感覺應該沒有內外部之分?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
其實我只是想表達從外部來還沒經過驗證的,我先改成 unvalidated
上面的已 fixed
@@ -43,8 +43,8 @@ public function custom_field_values_relationship_should_work(): void | |||
public function validate_custom_fields_should_work() | |||
{ | |||
$this->expectException(ValidationException::class); | |||
$this->user->zip_code = 123; | |||
$this->user->validateCustomFields(); | |||
$user = User::factory()->create(['account_id' => $this->account->id, 'custom' => ['zip_code' => 123]]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💬 comment
這邊呼叫create
的時候就會拋錯了,下面原本要測試的validateCustomFields
方法還會呼叫到嗎?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks! fixed!!!
…m key and load custom field value with the prefix custom
7e0899e
to
ea27ed2
Compare
No description provided.