Skip to content

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

Merged
merged 2 commits into from
Jun 27, 2023

Conversation

moon-brillar
Copy link
Contributor

No description provided.

@moon-brillar moon-brillar added the bug Something isn't working label Jun 27, 2023
@moon-brillar
Copy link
Contributor Author

Copy link
Contributor

@chenghung chenghung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link

@ericHao22 ericHao22 left a 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!!

@@ -46,13 +46,11 @@ public function validateCustomFields(): void
if ($customFields->isEmpty()) {
return;
}
$modelAttributeKeys = $modelAttributes->keys();
$customFieldColumns = $modelAttributeKeys->diff($tableColumns);
$externalCustomFields = $modelAttributes->get('custom');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ action require

可能要考慮到custom key拿到的值是null的情況,下面的validator在呼叫make的時候可能只接受array

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💬 comment

好奇為什麼這個變數要加上external的前綴?感覺應該沒有內外部之分?

Copy link
Contributor Author

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]]);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💬 comment

這邊呼叫create的時候就會拋錯了,下面原本要測試的validateCustomFields方法還會呼叫到嗎?

Copy link
Contributor Author

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
@moon-brillar moon-brillar force-pushed the 10-custom-fields-conflict-with-standard-columns branch from 7e0899e to ea27ed2 Compare June 27, 2023 07:40
@moon-brillar moon-brillar merged commit 83d6aad into main Jun 27, 2023
@moon-brillar moon-brillar deleted the 10-custom-fields-conflict-with-standard-columns branch June 27, 2023 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants