Skip to content
This repository has been archived by the owner. It is now read-only.

added getTranslationsArray() method #347

Merged
merged 5 commits into from Jun 6, 2017

Conversation

@askello
Copy link
Contributor

commented May 13, 2017

According to issue #273, it would be great to have a way to retrieve translations in the same structure such as for saving (like fill() method). For example I have a view with a form for updating some of my translatable records. My form fields named en[title], en[description], es[title] and so on. It is easy to store these data (via create() or fill() methods), and they can be easily passed to the view via $input->old() method. But there is no way to retrive and pass translations data to the view directly from translatable model, because their stucture is different. What do you think about it?

@Gummibeer
Copy link
Collaborator

left a comment

The method itself looks good and the feature introduced by it also.
But the unittest is no unittest for me - you test your logic with your copied logic. The assertEquals comparison array should be hard in the test to ensure that the method returns what we expect.
Or you split it in some smaller tests - test existing keys, test every single value etc.

askello added some commits May 15, 2017

Revert "fixed codestyle"
This reverts commit 23ef04d.
@unitedworx

This comment has been minimized.

Copy link
Contributor

commented May 17, 2017

it would be nice if this was also added in 6.* version which is the one compatible with 5.1 LTS

@dimsav dimsav merged commit ff2a07c into dimsav:master Jun 6, 2017

1 check passed

continuous-integration/styleci/pr The StyleCI analysis has passed
Details
@unitedworx

This comment has been minimized.

Copy link
Contributor

commented Jun 8, 2017

@askello i think the test for this method is flawed! i think you should use the same array to fill the model and then test if it equals with the array returned from getTranslationsArray() method! IMHO

@dimsav

This comment has been minimized.

Copy link
Owner

commented Jun 8, 2017

@unitedworx thanks for your feedback. I've fixed the test after merging.

@unitedworx

This comment has been minimized.

Copy link
Contributor

commented Jun 8, 2017

wow! am impressed :) wonderful work Dimitris

@askello

This comment has been minimized.

Copy link
Contributor Author

commented Jun 8, 2017

@unitedworx, you right. @dimsav, thank you for operativity!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants
You can’t perform that action at this time.