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

Added ability to auto-add fields to new fieldsets #12916

Merged
merged 17 commits into from
May 3, 2023

Conversation

snipe
Copy link
Owner

@snipe snipe commented Apr 26, 2023

This introduces two new (related) features:

Ability to add fields to field sets on fieldset create/edit

Screen.Recording.2023-04-25.at.9.35.39.PM.mov

Ability to auto-add fields to any new fieldsets that get created afterwards

Screen.Recording.2023-04-25.at.9.36.40.PM.mov

snipe added 13 commits April 25, 2023 15:51
Signed-off-by: snipe <snipe@snipe.net>
Signed-off-by: snipe <snipe@snipe.net>
Signed-off-by: snipe <snipe@snipe.net>
Signed-off-by: snipe <snipe@snipe.net>
Signed-off-by: snipe <snipe@snipe.net>
Signed-off-by: snipe <snipe@snipe.net>
Signed-off-by: snipe <snipe@snipe.net>
Signed-off-by: snipe <snipe@snipe.net>
Signed-off-by: snipe <snipe@snipe.net>
Signed-off-by: snipe <snipe@snipe.net>
…value

Signed-off-by: snipe <snipe@snipe.net>
Signed-off-by: snipe <snipe@snipe.net>
@what-the-diff
Copy link

what-the-diff bot commented Apr 26, 2023

PR Summary

  • Added auto-add feature and improved UI for Custom Fields
    Users can now automatically add an existing fieldset when creating or editing a new field. Fieldsets are displayed in a sidebar on small and medium screens, improving the layout consistency with other pages. Fixed a bug related to selecting all fieldsets.

  • Enhanced field syncing on new fieldsets
    When adding or editing fields, the system now syncs selected fieldsets with their respective ids, removing unselected ones from the pivot table.

  • Improved Custom Field usability
    The custom field delete button is now disabled when the field has a related set, preventing potential issues.

  • Added tests for CustomField::format() method
    Testing ensures the method returns the expected value based on its input parameter (IP, URL, or Email).

Copy link
Collaborator

@marcusmoore marcusmoore left a comment

Choose a reason for hiding this comment

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

This...I like this.... 👍🏾

app/Http/Requests/CustomFieldRequest.php Outdated Show resolved Hide resolved
Comment on lines +108 to +114
if ($fields->count() > 0) {
foreach ($fields as $field) {
$field_ids[] = $field->id;
}

$fieldset->fields()->sync($field_ids);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Side-note: since $fields is a collection you could skip the foreach and do

Suggested change
if ($fields->count() > 0) {
foreach ($fields as $field) {
$field_ids[] = $field->id;
}
$fieldset->fields()->sync($field_ids);
}
if ($fields->count() > 0) {
$fieldset->fields()->sync($fields->pluck('id'));
}

Copy link
Owner Author

Choose a reason for hiding this comment

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

Are we sure what will work as expected - it feels to me more like it would only associate that one it's syncing to, unsyncing the others (which is why I set it up as an array).

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it would be ok. $fields->pluck('id') will return a collection of the ids, which can be cast to an array, in the same way $field_ids[] is formatted.

Definitely don't hold up the PR for this comment though 😄

app/Http/Controllers/CustomFieldsController.php Outdated Show resolved Hide resolved
snipe added 2 commits May 3, 2023 10:55
Signed-off-by: snipe <snipe@snipe.net>
Signed-off-by: snipe <snipe@snipe.net>
@snipe snipe merged commit b716f9f into develop May 3, 2023
4 checks passed
@snipe snipe deleted the features/auto_add_to_fieldset branch May 3, 2023 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants