search

orchidsoftware / crud

Simplify the process of building CRUD (Create, Read, Update, Delete) functionality in Laravel using the features of Orchid.
https://orchid.software
MIT License
138 stars 34 forks source link

TD with query() that exposes builder #38

Closed bilogic closed 2 years ago

bilogic commented 3 years ago

Hi,

Referencing https://github.com/orchidsoftware/crud/issues/36, I added sort as well. Let me know if any other changes required. Thank you.

tabuna commented 3 years ago

Hi @bilogic. Please note that tests fail

bilogic commented 3 years ago

ohh.. ok thanks, let me check why.

tabuna commented 3 years ago

Can I help you with something so that we can integrate this?

bilogic commented 3 years ago

Yes of course, sorry I have been busy.

bilogic commented 3 years ago

Ok, I believe I got it fixed.

Just some feedback below:

  1. Code style: any tool you use? I'm looking at php-cs-fixer and aligned with Laravel's code style, otherwise contributors have to manually discard a lot unnecessary formatting before commiting. Maybe provide some instructions?
  2. Testing: I'm really quite new to testing, but wasn't able to set crud's up properly, I found that I don't have post table etc. In the end, I went through the logs on GitHub to pinpoint what the issue was. Maybe provide some instructions too?

Thank you!

bilogic commented 3 years ago

@tabuna anything else i need to do? Thanks.

tabuna commented 3 years ago

@bilogic Hi. I have local holidays now. I will be back in a couple of days and combine this

bilogic commented 3 years ago

@tabuna got it! enjoy! Thanks!

bilogic commented 3 years ago

Hi,

Does this need more changes?

bilogic commented 3 years ago

@tabuna can we merge this?

bilogic commented 3 years ago

@tabuna sorry, can we also make a decision on this? otherwise I have quite a bit of code hanging in the air. Thank you.

tabuna commented 3 years ago

Hi, I would like the code that affects the builder to be in the TD class, but I don't have enough time to change it myself. It would be great if you take it on yourself.

bilogic commented 3 years ago

Ok, I think I get what you mean, but if I'm going to move the logic into TD class, would it be better to move into https://github.com/orchidsoftware/platform's TD?

tabuna commented 3 years ago

According to the idea, there should be a hook/event that we will define in TD (from crud)