Follow-up to #2896229: move date handling to a processor, so date facets can use existing widgets for consistency. So that the settings can be changed, granularity can be accessed, from the config in the same way when constructing the query. Plus you could use other widgets.

Task: move granularity logic - like the date logic - to a processor, and possibly remove the numeric granular widget completely.
Also provide an upgrade path - just like in the date processor.

Let's also use this task to add tests for the granularity.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ekes created an issue. See original summary.

borisson_’s picture

Issue summary: View changes
borisson_’s picture

I ran all kernel and unit tests locally and had to fix one kernel test. We don't seem to have any functional tests for this though, making it somewhat less awesome to do this change.

No upgrade path yet.

borisson_’s picture

FileSize
566 bytes
4.88 KB

Needs config schema.

borisson_’s picture

Issue summary: View changes
Status: Active Β» Needs review
Issue tags: +Needs tests
FileSize
944 bytes
5.8 KB
borisson_’s picture

Issue tags: -Needs tests
FileSize
5.1 KB
10.38 KB

Adds an integration test as well.

Status: Needs review Β» Needs work

The last submitted patch, 6: move_numeric-2913329-6.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

borisson_’s picture

Status: Needs work Β» Needs review
FileSize
4.04 KB
13.08 KB

The integration test should now be green, it is locally. The unit test is still not passing but I have to leave. Would love to get input on the direction though.

Status: Needs review Β» Needs work

The last submitted patch, 8: move_numeric-2913329-8.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

borisson_’s picture

Status: Needs work Β» Needs review
FileSize
1 KB
13.13 KB

borisson_’s picture

Crediting d.novikov for his work in #2920982: Disallow '0' granularity when using "Granular numeric list" widget..

Integrated that in here + added form validation by setting the minimum value as an attribute.

Status: Needs review Β» Needs work

The last submitted patch, 12: move_numeric-2913329-11.patch, failed testing. View results

borisson_’s picture

Status: Needs work Β» Needs review
borisson_’s picture

Status: Needs review Β» Reviewed & tested by the community

Going to commit this later today.

  • borisson_ committed 3323ecf on 8.x-1.x
    Issue #2913329 by borisson_, ekes, d.novikov: Move numeric granularity...
borisson_’s picture

Status: Reviewed & tested by the community Β» Fixed

Committed, thanks!

Status: Fixed Β» Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.