-
Notifications
You must be signed in to change notification settings - Fork 5
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
Airtable Integration: Extract AirtableDatasource & leverage it for built-in & dynamic datasources #17
Conversation
use RemoteDataBlocks\Logging\Logger; | ||
use RemoteDataBlocks\Logging\LoggerManager; | ||
use RemoteDataBlocks\REST\DatasourceCRUD; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class should probably get moved out of the REST
namespace
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree.
8769930
to
b06aba5
Compare
…use instead of extending HttpDatasource themselves, support endpoint variations for tables in the former
735af4e
to
3206064
Compare
public function set_slug( string $slug ): void { | ||
$this->slug = $slug; | ||
} | ||
|
||
public function get_slug(): string { | ||
return $this->slug; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the purpose of the slug?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was meant to be the predictable unique identifier, so that we can reference the credentials in example code using this, and then users could tag the data source in the Settings Page with this slug. So the slug would act as a link between those 2 systems. Earlier we had uuid
as unique identifier, but that is not predictable until user adds the data source from Setting Page. Open to other ideas if they seem easier, this was just the first thought that popped into my mind.
… add/dynamic-block-registration
// Transform data to our experimental format, which is all array based | ||
$config = array_map( | ||
function ( $value ) { | ||
return is_object( $value ) ? (array) $value : $value; | ||
}, | ||
(array) $config | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was a hack when i was bumbling about in that unmerged try, but, at this point (hopefully) unnecessary.
(though I still think we should array everywhere personally!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests out for me. 👍
Much of this came in via #39, so I've limited it to abstracting the Airtable config.
The examples pull in a
AirtableDatasource
and provide a table "variation" (for lack of a better name -- any ideas to improve the API?) instead of hard-coding the endpoint.This lets us use a similar approach to dynamically register blocks for Airtable sources saved in the db, etc.