Custom Wizard should not override Application Route

As discussed on Discord.

PR complete here and ready for review:

@Angus, note I’ve removed what I believe to be redundant code too, please review carefully :slight_smile:

Could you please identify which code you think is redundant and why you think it’s redundant.

1 Like

Sure, np, this:

Reason: I don’t understand why this override is needed (and all tests pass without it).

btw, I tested this successfully in Production too using the “Users directed to wizard after start time” setting with my greatest invention yet, the “Boo!” Wizard:

Brilliant, huh? :smiley:

But let me tell you, it makes for quite a shock :open_mouth:

If you want a copy I’ll gladly provide the json … :wink:

1 Like

Please put that back in. It’s needed to ensure wizard redirects that get picked up by the DiscourseURL.routeTo helper (not all route transitions but some).

We don’t have any tests for that :wink:

Generally speaking leave things in if you don’t understand what they’re doing.

1 Like

We need to be careful not to build technical debt.

We really need tests on things so we know what they do and it is implicitly documented.

That said I appreciate integration code is very hard to add tests for.

I will add back.

1 Like

@Angus this is ready for review again.

And to make things easier to see, I’ve restored the original file structure so the changes are more obvious.

1 Like

This issue should be closed now: