Algo-Web / POData-Laravel

Composer Package to provide Odata functionality to Laravel
MIT License
34 stars 29 forks source link

Broken UTF-8 encoding #179

Closed Kirich11 closed 5 years ago

Kirich11 commented 5 years ago

Can't get correct display for strings image

c-harris commented 5 years ago

That's a new one. Probably an issue in Podata itself but I'll take a look in the next few days and let you know.

Thanks for reporting.

On Tue, 24 Sep. 2019, 2:10 am Kirichenko Nickita, notifications@github.com wrote:

Can't get correct display for strings [image: image] https://user-images.githubusercontent.com/18256217/65442912-bacada00-de35-11e9-89fa-477c3b3610ab.png

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Algo-Web/POData-Laravel/issues/179?email_source=notifications&email_token=AD5QEGRUE2LRFCNUUJAQ44TQLDS6VA5CNFSM4IZM2XL2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HNCSHNA, or mute the thread https://github.com/notifications/unsubscribe-auth/AD5QEGTKPMVZX6WPPPQKDCLQLDS6VANCNFSM4IZM2XLQ .

Kirich11 commented 5 years ago

There was a triple utf8 encoding, one was in IronicSerialiser so I've removed it https://github.com/Algo-Web/POData-Laravel/blob/master/src/Serialisers/IronicSerialiser.php#L877

I have non-english text, so maybe use mutlibyte conversions? Text is stored in a Mysql DB in utf8_general_ci encoding

c-harris commented 5 years ago

Well that saves me a lot of time tracking it down :) my only two languages are English and bad English so I have not tested other languages but we will take a look.

We are open to a pull request for mb encoding if you have the time. If not we will try and get to it as soon as possible.

On a side note I notice you have an "isPaid" field that is a string. Shouldn't that field be a bool?

On Tue, 24 Sep. 2019, 4:05 am Kirichenko Nickita, notifications@github.com wrote:

There was a triple utf8 encoding, one was in IronicSerialiser so I've remove it https://github.com/Algo-Web/POData-Laravel/blob/master/src/Serialisers/IronicSerialiser.php#L877 http://here

I have non-english text, so maybe use mutlibyte conversions? Text is stored in a Mysql DB in utf8_general_ci encoding

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Algo-Web/POData-Laravel/issues/179?email_source=notifications&email_token=AD5QEGRVLCIXM23CS7JHXRTQLEANTA5CNFSM4IZM2XL2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7LX2YY#issuecomment-534216035, or mute the thread https://github.com/notifications/unsubscribe-auth/AD5QEGXVOGIGRXK6VLKANSTQLEANTANCNFSM4IZM2XLQ .

Kirich11 commented 5 years ago

On a side note I notice you have an "isPaid" field that is a string. Shouldn't that field be a bool?

Indeed, it's an appended field and gets its value from a related model, maybe that's why it is a string

Kirich11 commented 5 years ago

So you were right =) The problem was in POData itself Here's a link to a PR https://github.com/Algo-Web/POData/pull/215

Main fix is in OutgoingResponse, I had to force charset in the content-type header

Gonna close this. Thx for your quick response. Feel free to do anything with PR. Glad to be helpful

c-harris commented 5 years ago

Thanks for that i will jump on the pull request shortly, Just FYI, if you put your attribute (appended field) in the $casts array on the model. PoData will read that. and case appropriately