It's currently too hard to switch the url processor, we should make that easier.

Todo

- make a dummy processor to hold urls
- make a new urlProcessor annotation / plugin type
- use the dummyprocessor to call out to the real url processor (that is set on the facet source)
- add tests
- bikeshed naming

Comments

borisson_ created an issue. See original summary.

ChristianAdamski’s picture

1.) The Default URL processor is currently locked, or at least it was a week ago in git.
2.) Taking the core_views_facet as an example, it requires a certain URL processor to work.
Vice versa, the default URL processor won't work there, and the search api facet source won't work with core_views_facets processor.

borisson_’s picture

Status: Active » Needs review
FileSize
44.09 KB

This should be the correct way to go, I think.

borisson_’s picture

Used git diff -M this time; patch is ~half the previous patch's size.

borisson_’s picture

Version: » 8.x-1.x-dev
FileSize
3.51 KB
44.65 KB

WIP

ChristianAdamski’s picture

Should I already go ahead and use this patch and align my code to it?

borisson_’s picture

Issue summary: View changes

Not yet, I discussed this a little bit in irc with StryKaizer and we both agree that there will be some big changes still to come in this patch.

borisson_’s picture

FileSize
4.43 KB
24.73 KB

Added more tests. I'm a confident this is a good solution, still needs more work and I'd appreciate a review.

borisson_’s picture

FileSize
8.55 KB
33.05 KB

MOAR TESTS.

borisson_’s picture

FileSize
7.52 KB
35.53 KB

Cleanup

StryKaizer’s picture

FileSize
3.18 KB
57.03 KB

Tested #10 with custom URL processor, works as expected.
Changed some docblocks and added a bit UI changes, see interdiff.

For me this can go in.

StryKaizer’s picture

Oh, we need tests first ofcourse.

For the naming part, I think URL processor handler is fine. There's a nice docblock which explains why that processor exists, and in the UI I just used URL Handler to simplify things.

borisson_’s picture

Issue summary: View changes
Status: Needs review » Fixed

Awesome, we might be able to add extra tests afterwards but there is coverage from an integration test + unit tests.

  • borisson_ committed 3704ea0 on 8.x-1.x
    Issue #2637594 by borisson_, StryKaizer: Make it easier to switch the...

  • borisson_ committed 8b7b203 on 8.x-1.x authored by StryKaizer
    Issue #2637594 by borisson_, StryKaizer: Make it easier to switch the...

The last submitted patch, 5: make_it_easier_to-2637594-5.patch, failed testing.

The last submitted patch, 8: make_it_easier_to-2637594-8.patch, failed testing.

The last submitted patch, 9: make_it_easier_to-2637594-9.patch, failed testing.

The last submitted patch, 10: make_it_easier_to-2637594-10.patch, failed testing.

Status: Fixed » Needs work

The last submitted patch, 11: make_it_easier_to-2637594-11.patch, failed testing.

borisson_’s picture

Status: Needs work » Fixed

Is already committed, closing up.

Status: Fixed » Closed (fixed)

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