allow adding columns in query field

[crissi]

Summary

Allow adding columns in query closure. Fixes: #875

People might be using addSelect() in their model relationship or have a need to ad an extra column to a query direcly in the custom query.

Type of change:

• Bug fix (non-breaking change which fixes an issue)

Checklist:

• [x ] Existing tests have been adapted and/or new tests have been added
• Add a CHANGELOG.md entry
• Update the README.md
• Code style has been fixed via composer fix-style

[mfn]

See my inline feedback, this is a BC break.

I can see the argument for your use case, but then we break the others and there's no "right or wrong" here. We currently support the one case and afterwards only the other.

Basically for this to work, we would need to keep the current 'query' and add another one, invoked afterwards.

Interestingly, now that your #799 is merged, you could extend SelectFields, override getSelectableFieldsAndRelations and make any logic you want…

What do you think? How do you want to proceed here? I can't merge "as is" as I don't see a good argument made for changing/dropping the existing behavuour.

[crissi]

Hi mfn.
I opened again using, addSelect and not reordering the way with and select works

[mfn]

Looks more elegant, but technically it's still change of behaviour…

Test coverage LGTM and I also can't think of case were this actually would break.

Are we sure we've no concerns here? Or should be rather wait for a major version bump?

We would also need a good / explanatory changelog for that one, so ppl understand the impact of the change.

WDYT?

[crissi]

could add it to this example: https://github.com/rebing/graphql-laravel#type-relationship-query
so it also included addSelect(

I personally dont think this can cause any errors.

[mfn]

could add it to this example

Ok, let's do this. Yes, please add it :)

[crissi]

could add it to this example

Ok, let's do this. Yes, please add it :)

added

[mfn]

Thank you!