WIP: V3 - refactoring #7

[pionl]

• upgrade code to PHP 8.0 with strict types
• reactor with automatic downgrade script
• phpstan 8 + phpcs fixer
• Upgrade queries / aggregations to use strict constructors
• Add GeoBoundingBoxQuery
• Add HistogramAggregation / WidthHistogramAggregation
• Add collapse support
• Improve sub aggregation on aggregations

[pionl]

Started refactoring my code base. At this moment most of the time is rewriting Filters to Query. Its not bad and its quite easy and the code looks pretty now :) 👍

  new BoolQuery(should: [
                new BoolQuery(filter: [
                    new RangeQuery(field: PriceTermsIndex::FROM, gte: $this->from->toDateString()),
                    new RangeQuery(field: PriceTermsIndex::TO, lte: $this->to->toDateString()),
                ]),
                // If the current year has no price set we want to return it as there was no price (will work if
                // HasNumberOfNights->allowTermsWithZeroNights is true)
                new TermQuery(PriceTermsIndex::NUMBER_OF_NIGHTS_RAW, 0),
            ]),

Aggregations were easy.

[erichard]

This is great ! Thanks :pray:

IMHO the code is even prettier when using the static factory ;)

[pionl]

Need to rename the branch from v3 (composer will not allow me to reference branch name with v in it). Do you know how to do it? Otherwise I will have to make a new PR.

Is the code ok for you? (the direction). Am I missing something?

[erichard]

You can rename your branch with git branch -m v3 refact-v3

The direction seems OK to me but he PR is quite large to review. Maybe you can split in mutliple PRs ? Like one for PHP 8.1 migration, one for rector, one for each new Query, etc

[pionl]

I did rename it but the PR got close but cant edit the PR branch.

About the code review - not sure if I can do it as proposed. It is mixed because rector / phpcs fixer improved the code quality.

• I can split the "proof of concept" of 7.4 / 7.2 code generation.
• I can split the new Query / aggregation (but don't see the time savings on code review).

Any way, how migrate my codebase (apx 50 filters -> queries) and it works 👍

[pionl]

I think im near to finishing all changes, I've made some tweaks I needed to be able to fully customize / use query builder.

I can try to separate the changes.

[erichard]

@pionl I've started to review this PR. One question so far : in the constructors sometime you set a private property and othertime a public one. Is there a particular reason for having both ? Shouldn't we use only private properties ?

[pionl]

@pionl I've started to review this PR. One question so far : in the constructors sometime you set a private property and othertime a public one. Is there a particular reason for having both ? Shouldn't we use only private properties ?

Public - I'll check which one and try to find usage. I think I've needed them in some cases (I'm doing some additional logic -> when this aggregation is used do this with source, etc).

Maybe we can expose all to public? Support set/get + "public property" ? Or use set/get everywhere? I can make the changes with your preference.

Thank you, Martin

[erichard]

Maybe we can expose all to public? Support set/get + "public property" ? Or use set/get everywhere? I can make the changes with your preference.

I prefer all privates with getter/setter, I think users will expect to have theses.

[erichard]

I can try to separate the changes.

No need to separate anymore, I reviewed the whole thing already :)

[pionl]

I prefer all privates with getter/setter, I think users will expect to have theses.

Ok I'll make the changes :)

[pionl]

Hi @erichard, I've made the changes, I've improved the git commits and added new StatsAggregation (this commit)

Anything else?

[pionl]

Also I've moved the "downgrade" proof of concept to different branch

[erichard]

Great work ! Thanks @pionl

[pionl]

Thank you for the cooperation!