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

Checks that custom fields exist before saving #14565

Draft
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

spencerrlongg
Copy link
Collaborator

Description

This adds a trait that checks that custom fields exist (and exist on the correct fieldset) before saving.

This cannot be merged in it's current form until #14458 is merged, so draft for now.

image

Fixes [sc-23936]

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Manually tested for now, custom fields are difficult to test at the moment (Marcus can speak to that better than I can)

Test Configuration:

  • PHP version: 8.1
  • MySQL version 8.2

Copy link

This pull request has been linked to Shortcut Story #23936: Better safeguards against arbitrary field names in API.

Copy link

what-the-diff bot commented Apr 8, 2024

PR Summary

  • Incorporation of Custom Field Trait in Store Asset Request
    The Store Asset Request now includes a trait called MayContainCustomFields. This enhances the system's capability to handle and process additional information contained in custom fields.

  • Introduction of a New File: MayContainCustomFields
    A new file called MayContainCustomFields.php has been added. This houses all the essential functions related to custom fields for more efficient and standardized custom field processing.

  • Addition of Validation Messages for Custom Fields
    New validation messages for custom fields have been included in validation.php. This ensures better error handling and user experience when errors occur associated with custom fields.

@spencerrlongg
Copy link
Collaborator Author

Was just curious and wanted to double check, this does actually merge with the error bag from the request's validator and it's the same validator instance, so all validation is included in the same response, which is pretty nice. 🙂

image

@snipe
Copy link
Owner

snipe commented Apr 11, 2024

Quick followup here - anything we can do to get this out of WIP?

@snipe snipe requested a review from uberbrady April 11, 2024 18:48
@uberbrady
Copy link
Collaborator

@spencerrlongg Can you re-target this to develop - maybe once you're a little further on out of WIP stage? Thank you!

@spencerrlongg spencerrlongg changed the base branch from master to develop April 17, 2024 13:54
@spencerrlongg
Copy link
Collaborator Author

Quick followup here - anything we can do to get this out of WIP?

We can talk about #14458, the PR that's holding this one in WIP.

Can you re-target this to develop - maybe once you're a little further on out of WIP stage? Thank you!

It's retargeted, it's only WIP to make sure it doesn't get merged without #14458 - other than that it's ready to go.

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

3 participants