Skip to content

Update proposal for custom attributes schema #456

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.

Sign up for GitHub

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

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

Update proposal for custom attributes schema #456

Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
11be876
Update custom attributes container proposal to account for attributes…
melnikovi Oct 20, 2020
72a2238
Update custom attributes container proposal to account for attributes…
melnikovi Oct 20, 2020
3205dd0
Update custom attributes container proposal to account for attributes…
melnikovi Oct 20, 2020
7a191b5
modifying proposal
cpartica Feb 5, 2021
c12eb3e
adding interfaces on inputs
cpartica Feb 10, 2021
c711235
refactor some things on product attributes
cpartica Feb 10, 2021
763fd6d
- refactor options
cpartica Feb 10, 2021
c2d29e2
- reactor types
cpartica Mar 5, 2021
27243fb
- refactor container explanation
cpartica Mar 5, 2021
49ed712
- rename option ids
cpartica Mar 5, 2021
74fd87e
refactoring to simplify query and schema
cpartica Mar 13, 2021
b365474
refactoring to simplify query and schema
cpartica Mar 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Update custom attributes container proposal to account for attributes… 
… with multiple values
  • Loading branch information
@melnikovi
melnikovi committed Oct 20, 2020
commit 3205dd0cbfbe59b77be530e414625dcfdc7b459b
4 changes: 2 additions & 2 deletions ...gn-documents/graph-ql/coverage/custom-attributes/custom-attributes-container.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ One workaround for "getting all fields" is based on schema introspection, it all

# Proposed solution

To account for dynamic nature of EAV attributes and the need of "getting all fields" in product search queries,we can introduce `custom_attributes: [CustomAttribute]!` container.
To account for dynamic nature of EAV attributes and the need of "getting all fields" in product search queries, we can introduce `custom_attributes: [CustomAttribute]!` container (recommended approach).
Copy link
Contributor

@cpartica cpartica Oct 22, 2020

Choose a reason for hiding this comment

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

do fragments bring any value for PWA since we still prefer these flat single structure arrays vs infterfaces?


```graphql
type CustomAttribute {
Copy link
Member

@mslabko mslabko Oct 20, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I think we have to add "type" opinion here to be able to render the appropriate template (datetime, text, multi-select, checkbox ...)

E.g.

"attributes":[
    {
      "code":"multiselect_attribute",
      "type":"multiselect",
      "values":[
            "Option 1",
            "Option 2"
      ]
    }
  ]

Copy link
Member Author

@melnikovi melnikovi Oct 20, 2020

Choose a reason for hiding this comment

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

Attributes are going to be displayed as information and not as fields you can control, so I don't think it's necessary.

Copy link
Member

@mslabko mslabko Oct 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

it will be useful to render field properly on UI without complex logic. E.g. for "Date" field - "10-11-2020"

Copy link
Contributor

@cpartica cpartica Feb 5, 2021

Choose a reason for hiding this comment

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

yes, they are needed for the forms

Copy link
Contributor

@paliarush paliarush Mar 29, 2021

Choose a reason for hiding this comment

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

We have this data already available in the scope of customAttributeMetadata query. Since update frequency for attribute metadata and product attribute values is different, it makes sense to separate them to avoid excessive payload size.

Expand All @@ -30,7 +30,7 @@ type CustomAttributeValue {
}
```

Alternative approach would be is to introduce an interface `custom_attributes: [CustomAttributeInterface]!`.
Alternative approach would be is to introduce an interface `custom_attributes: [CustomAttributeInterface]!`, but it seems more complicated.

```graphql
type CustomAttributeInterface {
Expand Down