-
Notifications
You must be signed in to change notification settings - Fork 15
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
To-do before first stable release #95
Comments
Hey, I so much love this bundle - Awesome work! I found it a while back and kept an eye on it, because I am eager to use it. I don't plan to use it with doctrine, we are using DTOs in our project anyway, so if doctrine moves out at ome point, it shouldn't affect it. The Array source is something that could be implemented via ProxyQuery if it's needed, but I don't see a usecase, i expect to build a lot of ProxyQuery classes myself to make it work in my project, so it also shouldn't affect me much. The symfony form integration is something I am not sure about. This might be a blocker if that changes at some point. So the question would be, how much effort and time is required to sort this one out? What worries me most is the theme task, because it's very sure I'll have to create my own theme and if it's subject to get changed soon, this could be a blocker for sure. This seems to be the most important task out of those 4, I guess? What do you think, how drastically can the changes be? will it void any prior effort to integrate this bundle if those tasks are done? Is there any roadmap ahead? |
Hey @CMH-Benny, sorry for the late reply and thank you for the kind words!
This bundle still requires a lot of work to consider it stable, including reworking existing parts to make them testable (no coverage yet for most crucial parts of the library). It may work fine in some cases, but the amount of possible breaking changes to come may not be worth to install it yet, especially considering that your use case seems to require more than a simple table for a CRUD :) I am really happy that you're interested in the bundle and consider it useful, but I think I am not able to say confidently that in the current state it wouldn't introduce more problems that solutions. Especially considering my limited time developing it in the last months, but I hope that would change soon (side note - I am not abandoning this project or something like that, just things been rough lately).
Please note, that
This will introduce many breaking changes due to some changes in the internal interfaces, such as
There will be changes in block naming, HTML attribute inheritance between blocks, and I think that's it for now. This may be a blocker for sure when using custom themes that override some parts of the built-in ones.
That is really hard to say. There will be many breaking changes, although I am always trying to write down notes on how to update existing applications (as my co-workers are already using this bundle on daily basis, but mostly on CRUD applications based on Doctrine). Currently, I cannot be confident that new releases does not introduce even most trivial bugs due to the lack of tests. In summary, I think, in your case (from the description you provided) I would recommend not using this bundle yet. Maybe on some projects with smaller or simpler scope just to test things out, and wait for a stable version. Not sure when that will be though - I feel like this is still something barely more than proof of concept, sometimes unstable, but usable in cases where overriding internal classes is not required. Or maybe I'm exaggerating, as this is my first open source project of such scale. Cheers! |
Thank you so much for your answer, that is actually not what I wanted to hear :D I digged into the docs and this package is already really amazing, so I didn't want to write all that myself. I think such a kind of abstraction would be nice to have out of the box, so that there is room for different "bridges" not only for doctrine, but maybe also for elastic search, api calls, reading files from storage etc. If that would happen, maybe we could try to get this package into symfony-ux itself? That would be awesome, there are many people with a need for such a powerful datatable package :) I probably will get in trouble, if there are breaking changes at some point, but hopefully, due to that abstraction I only need to adjust it on that transformation layer. Especially the filters worry me, but at least I translate them into my own set of classes, so from there I am safe. 🤞 In regards of the template/theme I need to see. If block names of the template gets changed, this might cause issues, but since I very likely need to fully rewrite the theme, maybe it won't be too hard to adjust later. Since there is no real base for a tailwind I need to kinda start from scratch and I would guess changing some block names later won't be such a big deal. Maybe I will even come up with my own blocks, didn't spend much time on that yet :D I would like to contribute, but for now I am playing around and I am not sure if i will have the time, but I would so much love to see a ready to use datatable packge for modern symfony applications :) Thank you so much |
Isn't the abstraction with
Great to hear. If you encounter any problems, or just need with that, let's create a discussion thread, and I'll try my best to help you 😄
Regarding to Tailwind, it'll be easier to extend the |
As of right now, it seemed to be not enough for me, the ProxQueryInterface only enforces methods for paginate and sort, but not to add any filters, this is only implemented in your DoctrineQuery implementation. What I did is, I created a Hope I was able to explain it good enough :) But in tl;dr it felt like the ProxyQueryInterface is not sufficient by only asking for sort and paginate, while the whole filtering is kinda hardcoded for Doctrine only :)
Good idea, if we want to continue the talk, this might make sense, don't want to pollute this issue here, I added this answer now, because it also contians some kind of feedbackm but we can move to a discussion if you want :)
That is good to hear and I guess I will do so, still not sure when exactly I will tacle this, but probably soon :D Thank you again and have a nice day 🙌 |
hey, maybe create a demo github repository? :) |
Good idea. Similar to EasyAdmin demo application, we could base it off the official Symfony demo. |
Hey @CMH-Benny, coming back to this topic to give a small hint. We can continue here, this issue is mostly for the discussion anyway :)
If I understand what you mean correctly, in your case, you cannot apply the filters individually, but rather collect the filtration data and apply it only once. I'm thinking about using an external API, that requires simply sending a single request at the end, with specific page, limit, sorting and filtration. You don't need additional classes for that, just create a single ProxyQuery class for your data source, and implement a custom method (like class CustomDataSourceFilterHandler implements FilterHandlerInterface
{
public function handle(ProxyQueryInterface $query, FilterData $data, FilterInterface $filter)
{
$query->addFilter($data, $filter);
}
}
class CustomDataSourceProxyQuery implements ProxyQueryInterface
{
public function paginate(PaginationData $data) {
$this->paginationData = $data;
}
public function sort(SortingData $data) {
$this->sortingData = $data;
}
public function addFilter(FilterData $data, FilterInterface $filter) {
// now im just simplifying this as much as possible but you get the idea
// of collecting filters to apply in the getResult(), instead of doing it in the handler
$this->filters[$filter->getName()] = $data;
}
public function getResult() {
// retrieve a data with applied pagination, sorting and filtration...
}
} I think you described something like this, but I really don't understand what's the reason for the two additional interfaces (
I don't understand where the "hardcoded" part is coming from If I missed something, please let me know. I don't fully understand your case, and I would really appreciate any feedback on why you see the current implementation as something tied to Doctrine, or not flexible enough (I plan on adding a comprehensive article about how to integrate with custom data sources to the documentation, so any issues you encounter would be valuable) |
The class CustomDataSourceFilterHandler implements FilterHandlerInterface
{
public function handle(ProxyQueryInterface $query, FilterData $data, FilterInterface $filter)
{
if (!$query instanceof DataProviderQueryInterface) {
throw new RuntimeException('Query must be an instance of DataProviderQueryInterface');
}
$query->addFilter($data, $filter); // addFilter is now 100% existing, because handle expects a different type of interface
}
} And because of that interface, I can resuse filters for every DataProviderQuery And the interface DataTableDataProviderInterface
{
public function getData(DataTableQueryInterface $query): DataTablePageResultSetInterface;
} I can now implement my custom provider: readonly class CustomProvider implements DataTableDataProviderInterface
{
public function __construct(
private SomeService $someService,
) {
}
public function getData(DataTableQueryInterface $query): DataTablePageResultSetInterface
{
$options = [];
if ($query->getPagination()) {
$options['pagination'] = [
'page' => $query->getPagination()->getPage(),
'limit' => $query->getPagination()->getPageSize(),
];
}
if ($query->getSearchTerm()) {
$options['search'] = $query->getSearchTerm();
}
if ($query->getSorting()) {
$sorting = $query->getSorting()[0];
$options['sort'] = [$sorting->getPropertyPath() . ':' . ($sorting->getDirection()?->value ?? 'asc')];
}
if ($query->getFilters()) {
foreach ($query->getFilters() as $filter) {
// this still needs to go to somewhere more generic to not have to reimplement it all the time
match (get_class($filter)) {
NumberFilter::class => $filter->getNumber() ? $options[$filter->getPropertyPath()] = $filter->getNumber() : null,
TextFilter::class => $filter->getText() ? $options[$filter->getPropertyPath()] = $filter->getText() : null,
DateFilter::class => $filter->getDate() ? $options[$filter->getPropertyPath()] = $filter->getDate()->format('Y-m-d') : null,
Filter::class => $options[$filter->getPropertyPath()] = $filter->getValues(),
default => null,
};
}
}
# shape of this query is set inside Finder
$result = $this->someservice->doSomething($options);
return new DataTablePageResultSet(
$result->getData(),
$result->getPage(),
$result->getLimit(),
$result->getResultCount(),
$result->getTotalCount(),
);
}
} So with my provider, I can use any service, a doctrine repo, an elatic search index, opening a file handle, call to an API or whatever and all I need to do is map it back to the proper result class and it will work. That way I don't need to write a new ProxyQueryInterface implementation for each new table, I only need to implement a DataTableDataProvider And the best thing is, when I pass such an provider in the controller as second argument, it checks for this interface and automatically turns it into my DataProviderQuery (through a
I mean hardcoded, because it only works with a Doctrine Query Builder and this handling has no interface so it just offers additional methods is magicaly uses and if you just use doctrine directly in your controller this probably is fine, but we never use doctrine directly, we have custom repository classes and hide away the query builder of it etc. so for me it was important to get the data into a service that can do the transformation/translation between datatable and some other datasource |
Just for reference - we really need more code coverage for critical parts of the bundle.
Additionally:
If you have any problems with the bundle, please, let me know. My contributions slowed down a little this year, but I'll try my best to move forward with the releases.
The text was updated successfully, but these errors were encountered: