Preloaded API document Improvements #2754

askvortsov1 · 2021-04-04T19:15:42Z

Fixes #2752

Changes proposed in this pull request:

Fix getting the handler for custom routes, add automated tests to confirm.
Invalidate preloadedApiDocument in frontend if URL has changed since page load

Reviewers should focus on:

I feel like we might want to make the workaround I included to configure settings before boot as an official method of the testing package. That's probably best as a separate PR though.

Confirmed

Frontend changes: tested on a local Flarum installation.
Backend changes: tests are green (run composer test).

Required changes:

Related documentation PR: (Remove if irrelevant)
Related core extension PRs: (Remove if irrelevant)

Also add a test ensuring that the proper handler is used when default route is overriden.

[ci skip] [skip ci]

src/Forum/ForumServiceProvider.php

[ci skip] [skip ci]

SychO9

Should we wait to use $this->setting() or should we do that in a different pull request ? it would make sense in a different pull request of there are other places we can use it, which I believe is the case?

askvortsov1 · 2021-04-07T22:18:41Z

Should we wait to use $this->setting() or should we do that in a different pull request ? it would make sense in a different pull request of there are other places we can use it, which I believe is the case?

Let's do a separate PR, we'd need to update the used version of flarum/testing, which should definitely not be in this PR.

xiaowu771086986 · 2021-05-11T13:25:19Z

src/Forum/ForumServiceProvider.php

@@ -204,7 +204,7 @@ protected function setDefaultRoute(RouteCollection $routes)
        $factory = $this->container->make(RouteHandlerFactory::class);
        $defaultRoute = $this->container->make('flarum.settings')->get('default_route');

-        if (isset($routes->getRoutes()['GET'][$defaultRoute]['handler'])) {
+        if (isset($routes->getRouteData()[0]['GET'][$defaultRoute]['handler'])) {
            $toDefaultController = $routes->getRoutes()['GET'][$defaultRoute]['handler'];


$toDefaultController = $routes->getRoutes()['GET'][$defaultRoute]['handler'];
$toDefaultController = $routes->getRouteData()[0]['GET'][$defaultRoute]['handler'];
Does it need to be modified ？

Hello @xiaowu771086986,

Yea we hadn't noticed it at the time and tests didn't catch it, but we did actually fix it not long after here: 1cca4e8

Is this what you meant ?

yes ,thank you !

askvortsov1 and others added 3 commits April 4, 2021 15:03

Fix getting handler for the default route

84f5892

Also add a test ensuring that the proper handler is used when default route is overriden.

Invalidate preloadedApiDocument if URL has changed

3f2bc3f

Apply fixes from StyleCI

fe61e66

[ci skip] [skip ci]

askvortsov1 requested review from SychO9 and tankerkiller125 April 4, 2021 19:15

tankerkiller125 approved these changes Apr 4, 2021

View reviewed changes

SychO9 reviewed Apr 7, 2021

View reviewed changes

src/Forum/ForumServiceProvider.php Outdated Show resolved Hide resolved

askvortsov1 and others added 2 commits April 7, 2021 16:01

Revert to using getRouteData()[0]

4754e5d

Apply fixes from StyleCI

c84f5da

[ci skip] [skip ci]

askvortsov1 requested a review from SychO9 April 7, 2021 21:22

SychO9 approved these changes Apr 7, 2021

View reviewed changes

SychO9 merged commit 40dc6d0 into master Apr 7, 2021

SychO9 deleted the as/preloaded-api-document-improvements branch April 7, 2021 22:25

SychO9 referenced this pull request Apr 7, 2021

Fix a missed getRouteData()

1cca4e8

SychO9 mentioned this pull request Apr 7, 2021

Fix a missed getRouteData() #2774

Merged

1 task

xiaowu771086986 reviewed May 11, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Preloaded API document Improvements #2754

Preloaded API document Improvements #2754

askvortsov1 commented Apr 4, 2021

SychO9 left a comment

askvortsov1 commented Apr 7, 2021

xiaowu771086986 May 11, 2021

SychO9 May 11, 2021

xiaowu771086986 May 12, 2021

Preloaded API document Improvements #2754

Preloaded API document Improvements #2754

Conversation

askvortsov1 commented Apr 4, 2021

SychO9 left a comment

Choose a reason for hiding this comment

askvortsov1 commented Apr 7, 2021

xiaowu771086986 May 11, 2021

Choose a reason for hiding this comment

SychO9 May 11, 2021

Choose a reason for hiding this comment

xiaowu771086986 May 12, 2021

Choose a reason for hiding this comment