eloquent-driver
eloquent-driver copied to clipboard
statamic
Preserve order from CP when saving entries
This goes some of the way to resolving issue #43 in that it preserves the order, however if we're using incrementing IDs then ordering will fail once IDs hit double digits as it will treat it as string ordering rather than numeric ordering (so 1, 10, 2, 3 etc).
I'm not sure how to resolve this in the JSON field with getting into specific code for specific database types.
The best option might be to make an order column in the database for handling this?
Update: I've tapped into laravel core builder methods to cast the order as a real field which seems to do the trick.
Hi , i tried this pr again and i have time format issue while reordering , is it something to do with my mysql configuration ?
If you can supply a sample repository with clear steps to reproduce the issue then I can look into it further.
On 16 Sep 2022, at 15:28, Enes Polat @.***> wrote:
https://user-images.githubusercontent.com/21336979/190662690-188aa983-f89c-4e1b-b746-115c65c60558.pngHi , i tried this pr again and i have time format issue while reordering , is it something to do with my mysql configuration ? — Reply to this email directly, view it on GitHub https://github.com/statamic/eloquent-driver/pull/57#issuecomment-1249437227, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAMVOZ2NLSC3OLZM5JMEPLV6R72RANCNFSM6AAAAAAQFE2MXA. You are receiving this because you authored the thread.
i'll try to supply a sample tonight but i temporarily eleminated the problem like this in the CollectionRepository
That shouldnt be necessary as updated_at should come from the laravel model timestamps.
@enespolat24 can you create a sample repository so I can look into this further?
@jasonvarga this is good to merge from our testing and the feedback from others
@ryanmitchell I tested this with one of my live projects. We use mariaDb instead of mysql. Can we use DOUBLE PRECISION instead of REAL since mariadb doesn't support REALdatatype ?
I checked their docs. they all support DOUBLE PRECISION but in the Postgres there's a storage size difference. REAL uses 4 byte but DOUBLE PRECISION uses 8 byte
Ok. I ran some quick tests and it seems ok to switch to that.
On 28 Sep 2022, at 16:02, Enes Polat @.***> wrote:
I checked their docs. they all support DOUBLE PRECISION but in the Postgres there's a storage size difference. REAL uses 4 byte but DOUBLE PRECISION uses 8 byte
— Reply to this email directly, view it on GitHub https://github.com/statamic/eloquent-driver/pull/57#issuecomment-1261047140, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAMVO66USZ3QH2T5QUWKYLWARMY3ANCNFSM6AAAAAAQFE2MXA. You are receiving this because you were mentioned.
@jasonvarga would be great to get this merged as ordering isn't working in the CP at the moment.
@enespolat24 Does this PR fix your issue? Last you said on #43 was that you're still having issues.
@ryanmitchell What's the reason for using the orderByRaw and CAST stuff? Is it because you don't want to add an order column?
I'm using MariaDB 10.5.8 and it doesn't seem to like that syntax. I get the following error when I visit the reorder entries screen in the CP.
SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'PRECISION) ASC' at line 1
select
*
from
`entries`
where
`collection` = 'more_pages'
order by
CAST(
json_unquote(json_extract(`data`, '$."order"')) AS DOUBLE PRECISION
) ASC
limit
50 offset 0
@jasonvarga works like a charm with mysql but i tried this with our Mariadb 10.6.3 and got same error as you do. the walk around i did was changing DOUBLE PRECISION as DOUBLE. Still not optimum but does the job. with this temporary solution i need to click save button more than once
Yeah I was trying to avoid adding a column but I guess in hindsight it’s just making things more complicated. I’ll refactor it to use a nullable order column
Two things:
- Why are you calling the column
sort_orderinstead of justorder? - How can we get that migration going? It's just a stub, how are people supposed to use it once they upgrade?
Order is a reserved word in some db engines. I’ve always avoided using it as a result.
Yeah the migrations annoyed me too - I don’t like the approach of publishing then every time but others contributing to the big PR wanted it so I gave in.
To me it should automatically publish but I think we are stuck now?
Order is a reserved word in some db engines. I’ve always avoided using it as a result.
I would think as long as its quoted appropriately it'll be fine with it being a column name.
ie. select `title`,`order` from `entries` as opposed to just select title,order from entries
Laravel adds the quotes.
As for publishing the migrations.... even if you publish the migrations again, it publishes all of them, not just new ones. 🤔
Ok I’ll change it over later on.
Yeah I should have really pushed back on the migrations, I knew it wasn’t the right approach - sorry about that.
do we need to do something like the upgrade to logic in core that you use for adding permissions etc?
Thanks. We'll figure out the migration side of it from here. Leave it with us.
Much appreciated!