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

CommentFileSizeAuthor
#11 make_it_easier_to-2637594-11.patch57.03 KBstrykaizer
#11 interdiff.txt3.18 KBstrykaizer
#10 make_it_easier_to-2637594-10.patch35.53 KBborisson_
#10 interdiff.txt7.52 KBborisson_
#9 make_it_easier_to-2637594-9.patch33.05 KBborisson_
#9 interdiff.txt8.55 KBborisson_
#8 make_it_easier_to-2637594-8.patch24.73 KBborisson_
#8 interdiff.txt4.43 KBborisson_
#5 make_it_easier_to-2637594-5.patch44.65 KBborisson_
#5 interdiff.txt3.51 KBborisson_
#4 make_it_easier_to-2637594-4.patch21.22 KBborisson_
#3 make_it_easier_to-2637594-3.patch44.09 KBborisson_

Comments

borisson_ created an issue. See original summary.

christianadamski’s picture

christianadamski commented

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

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented
Status: Active » Needs review
StatusFileSize
new make_it_easier_to-2637594-3.patch44.09 KB

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

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented
StatusFileSize
new make_it_easier_to-2637594-4.patch21.22 KB

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

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented
Version: » 8.x-1.x-dev
StatusFileSize
new interdiff.txt3.51 KB
new make_it_easier_to-2637594-5.patch44.65 KB

WIP

christianadamski’s picture

christianadamski commented

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

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented
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

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented
StatusFileSize
new interdiff.txt4.43 KB
new make_it_easier_to-2637594-8.patch24.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

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented
StatusFileSize
new interdiff.txt8.55 KB
new make_it_easier_to-2637594-9.patch33.05 KB

MOAR TESTS.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented
StatusFileSize
new interdiff.txt7.52 KB
new make_it_easier_to-2637594-10.patch35.53 KB

Cleanup

strykaizer’s picture

strykaizer
Primary language Dutch
Location Belgium
commented
StatusFileSize
new interdiff.txt3.18 KB
new make_it_easier_to-2637594-11.patch57.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

strykaizer
Primary language Dutch
Location Belgium
commented

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

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented
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

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented
Status: Needs work » Fixed

Is already committed, closing up.

Status: Fixed » Closed (fixed)

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