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

feat: Automatically include 'id' field in related table queries #973

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

inmass
Copy link

@inmass inmass commented Sep 23, 2024

(this may be also related to issue #959

Automatically Include 'id' Field in Related Table Queries

Description

This PR modifies the getRequestedFieldsForRelatedTable method to automatically include the 'id' field when querying related tables. This enhancement improves user experience by reducing the need for redundant specifications in API requests.

Motivation and Context

When working with related tables, the 'id' field is often crucial for maintaining relationships and performing subsequent operations. However, requiring users to explicitly include 'id' in every request (e.g., fields[carModel]=slug,id) can be cumbersome and error-prone. This change aims to streamline the API usage while ensuring that essential data is always available.

@AlexVanderbist
Copy link
Member

Hi @inmass, thanks for another interesting contribution! I agree that it's not the best DX to include the relevant id columns on included fields all the time. However, I never found a good fix for this because there's a bunch of edge cases where it's not as simple as adding a select on id. For example, some people use uuid as a key, while others use custom foreign key columns like related_id. There are also special relationship types, such as polymorphic relations, which require more than one column to clarify the relationship. Maybe I see the issue as more complicated than it actually is, but let's get some test cases going first to see where this breaks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants