-
-
Notifications
You must be signed in to change notification settings - Fork 338
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
API support for "Link Table" dialog #972
Comments
Assigned to myself to review. |
Added some ideas about how to design the API. |
@seancolsen I have described a rough API schema below, please do suggest changes if any. // /api/v0/links/
[
{
"link_type": "o2o", // One to One
"data": {
"referent_column_id": number,
"column": { // Data to create new column
name
// ... other column data
},
"column_id": number // Use existing column object
}
},
{
"link_type": "m2m", // Many to Many
"data": {
"referent_columns": [number],
"map_table": {
// ... other table data
},
}
] |
Hmmm. Now that I'm reading this ticket more closely, I'm a bit confused about some of the details in the "Ideas" section that @kgodey wrote in the ticket description. I apologize that I didn't catch this sooner and raise concern with it or propose an API spec. As far as I can tell, we don't have a need to use a special API like the "links" API to list or delete links. And those operations can be somewhat complex if we choose to do them through this sort of API. (I can provide examples of such complexity if needed.) I was originally envisioning this API as serving only the purpose of creating links. I'd like to understand if there's a use-case that I'm not seeing though. With the scope of this API limited to only the use-case that I can currently envision the front end requiring, here's an alternate proposal for this API... Proposal B
|
A few points, not all related to each other:
That being said, we can always refactor the API if needed. I've got a bunch of feedback about @silentninja's API proposal too, but I'm not sure if it would be useful to give it now. |
@seancolsen @kgodey I have listed my proposed API schema below, would like to get a feedback on it. interface OneToOneRequestParams {
/** base table */
referent_table_id: number;
/** reference table */
reference_table_id: number;
reference_column: {
name: string;
}
}
interface ManyToManyParams {
mapping_table: string;
reference_objects: [{
reference_table_id: number;
reference_column: string;
}]
}
interface LinkTablePostRequestParams {
link_type: 'one-to-one' | 'one-to-many' | 'many-to-one' | 'many-to-many'; // Need this for validation
data: OneToOneRequestParams | ManyToManyParams
}
/** Response */
interface OneToOneResponseParams {
/** base table */
referent_table_id: number;
/** reference table */
reference_table_id: number;
reference_column_id: number;
}
interface ManyToManyResponseParams {
mapping_table_id: number;
reference_objects: [{
reference_table_id: number;
reference_column_id: number;
}]
}
interface LinkTablePostResponseParams {
link_type: 'one-to-one' | 'one-to-many' | 'many-to-one' | 'many-to-many';
data: OneToOneResponseParams | ManyToManyResponseParams
} |
@silentninja I'm not sure how to read this API schema, I'll leave it to you and @seancolsen to figure out. I think you should do whatever is fastest and we can figure out the best pattern later, during the cooldown. You can add it to the weekly meeting agenda for issues to figure out like @seancolsen has been doing. |
@silentninja Your proposal seems pretty similar to mine, so I'm glad to see that we seem to be close to working this out. I don't envision ever needing to utilize the response from this API, so I'll continue my critique by focusing only on the request. Critique:
With the above critique in-mind, here is my counter-proposal which builds on yours, offering some minor adjustments to the schema and fixes to the TS syntax. Proposal Dinterface OneToOneOrManyToOne {
link_type: 'one-to-one' | 'many-to-one';
data: {
reference_table_id: number;
reference_column: { name: string };
referent_table_id: number;
}
}
interface OneToMany {
link_type: 'one-to-many';
data: {
reference_table_id: number;
referent_table_id: number;
referent_column: { name: string }; // <- See note (1)
};
}
interface ManyToMany {
link_type: 'many-to-many';
data: {
mapping_table_name: string; // <- Name adjusted for clarity
referents: { // <- Name adjusted for simplicity
table_id: number; // <- Name adjusted for simplicity
column_name: string; // <- Name adjusted for simplicity/clarity
}[] // <- See note (2)
};
}
type LinkTablePostRequest = OneToOneOrManyToOne | OneToMany | ManyToMany; Notes:
Special validation to consider:
|
Problem
In the "Link Table" dialog, the front end needs to create a new foreign key reference between two tables.
The front end can already perform all of those operations through the API. However, at the DB level, we'de like for those operations to occur within the same transaction, which necessitates some additional API features.
Solution
The implementer should write a proposed API spec and get feedback from other members of the engineering team before implementing this issue.
Ideas
This section written by @kgodey.
/api/v0/links/
endpoint that lists all links and their types and allows creation and deletion of links. Creating and delete links should encapsulate all operations involved (creation and deletion of columns, constraints, and/or tables) in a single transaction.Details about how to define many-to-many and many-to-one tables need to be worked out as part of the spec. Please also consider the case of tables with FKs to themselves while doing this.
Additional context
The text was updated successfully, but these errors were encountered: