IQBall
/
Application-Web
6
0
Fork 0
Code Issues 49 Pull Requests Packages Projects 5 Releases Wiki Activity
Labels Milestones

Fix staging server deployment #24

Merged
maxime.batista merged 2 commits from staging/fix into master 2 years ago
Conversation 0 Commits 2 Files Changed 21
maxime.batista commented 2 years ago

maxou.dev/IQBall is currently not serving pages correctly because we directly use the routes as is in our twig views. For example :

(in error.html.twig)

<button class="button" onclick="location.href='/home'" type="button">Retour à la page d'accueil</button>

The button will redirect to /home, which works in our local development servers, but not in our staging server, as there is a base path to place before :
https://maxou.dev/staging/fix/public/home

As you can see, to access to the home view on staging server, we must specify a base path before.
the base path is constitued as is : /<branch>/public, see Documentation/how-to-dev.md for more information.

To fix this, i added into the twig environment, the global variable basePath, which is to prepend before each server urls.

previous example updated :

<button class="button" onclick="location.href='{{ basePath }}/home'" type="button">Retour à la page d'accueil</button>
`maxou.dev/IQBall` is currently not serving pages correctly because we directly use the routes as is in our twig views. For example : (in error.html.twig) ```html <button class="button" onclick="location.href='/home'" type="button">Retour à la page d'accueil</button> ``` The button will redirect to `/home`, which works in our local development servers, but not in our staging server, as there is a base path to place before : https://maxou.dev/staging/fix/public/home As you can see, to access to the home view on staging server, we must specify a base path before. the base path is constitued as is : `/<branch>/public`, see `Documentation/how-to-dev.md` for more information. To fix this, i added into the twig environment, the global variable `basePath`, which is to prepend before __each__ server urls. previous example updated : ```html <button class="button" onclick="location.href='{{ basePath }}/home'" type="button">Retour à la page d'accueil</button> ```
maxime.batista added 1 commit 2 years ago
maxime.batista requested review from clement.freville2 2 years ago
maxime.batista requested review from yanis.dahmane-bounoua 2 years ago
maxime.batista requested review from vivien.dufour 2 years ago
maxime.batista requested review from mael.daim 2 years ago
maxime.batista requested review from samuel.berion 2 years ago
clement.freville2 approved these changes 2 years ago
clement.freville2 left a comment

This is a valid fix for the issue. In the long term however, you may be interested in exposing functions that create the path instead of relying on the view to "hopefully not forget how to concatenate correctly".
Paths are handled by the router, and it can be used to generate routes given a route name. This may also be interesting to throw an error if the route is defined or to properly encode URL parameters.

This is a valid fix for the issue. In the long term however, you may be interested in exposing functions that create the path instead of relying on the view to "hopefully not forget how to concatenate correctly". Paths are handled by the router, and it can be used to [generate routes given a route name](https://github.com/dannyvankooten/AltoRouter/blob/4270bb5ca203cd958d4866b797d7bc00e16b4905/AltoRouter.php#L143). This may also be interesting to throw an error if the route is defined or to properly encode URL parameters.
public/index.php
$fl = new FilesystemLoader("../src/App/Views");
$twig = new Environment($fl);
$twig->addGlobal("basePath", $basePath);
clement.freville2 commented 2 years ago

Because path concatenation cannot always be resumed to a simple string concatenation, exposing a path(string) function would hide this base path.

use Twig\Extension\AbstractExtension;
use Twig\TwigFunction;

class PathExtension extends AbstractExtension {
    private string $basePath;

    public function __construct(string $basePath) {
        $this->basePath = $basePath;
    }

    /**
     * @return TwigFunction[]
     */
    public function getFunctions(): array {
        return [new TwigFunction('path', fn(string $path): string => $this->basePath . '/' . $path)];
    }
}

// Register the extension
$twig->addExtension(new PathExtension($basePath));

// Or just register use this shortcut:
$twig->addFunction(new TwigFunction('path', fn(string $path): string => $this->basePath . '/' . $path));

<a class="button" href="{{ path('home') }}">Retour à la page d'accueil</a>
Because path concatenation cannot always be resumed to a simple string concatenation, exposing a `path(string)` function would hide this base path. ```php use Twig\Extension\AbstractExtension; use Twig\TwigFunction; class PathExtension extends AbstractExtension { private string $basePath; public function __construct(string $basePath) { $this->basePath = $basePath; } /** * @return TwigFunction[] */ public function getFunctions(): array { return [new TwigFunction('path', fn(string $path): string => $this->basePath . '/' . $path)]; } } ``` ```php // Register the extension $twig->addExtension(new PathExtension($basePath)); // Or just register use this shortcut: $twig->addFunction(new TwigFunction('path', fn(string $path): string => $this->basePath . '/' . $path)); ``` ```twig <a class="button" href="{{ path('home') }}">Retour à la page d'accueil</a> ```
maxime.batista marked this conversation as resolved
src/Api/API.php Outdated
header('Content-type: application/json');
echo $response->getJson();
} else {
throw new Exception("API returned a non-json response.");
clement.freville2 commented 2 years ago

This change is unrelated, and it is better not to silence such errors.

This change is unrelated, and it is better not to silence such errors.
maxime.batista marked this conversation as resolved
vivien.dufour approved these changes 2 years ago
maxime.batista force-pushed staging/fix from cc4f60be12 to fffb520dbf 2 years ago
maxime.batista merged commit 2e26056ae9 into master 2 years ago
maxime.batista deleted branch staging/fix 2 years ago

Reviewers

mael.daim was requested for review 2 years ago
yanis.dahmane-bounoua was requested for review 2 years ago
samuel.berion was requested for review 2 years ago
clement.freville2 approved these changes 2 years ago
vivien.dufour approved these changes 2 years ago
continuous-integration/drone/push Build is passing
Details
The pull request has been merged as 2e26056ae9.
Sign in to join this conversation.
Reviewers
Request review
No reviewers
mael.daim
yanis.dahmane-bounoua
samuel.berion
clement.freville2
vivien.dufour
Labels
Apply labels
Clear labels
bug
Something is not working duplicate
This issue or pull request already exists enhancement
New feature help wanted
Need some help invalid
Something is wrong question
More information is needed wontfix
This won't be fixed
No Label bug duplicate enhancement help wanted invalid question wontfix
Milestone
Set milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Assign users
Clear assignees
No Assignees
3 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: IQBall/Application-Web#24
Write Preview
Loading…
Cancel
Save
There is no content yet.
Delete Branch 'staging/fix'

Deleting a branch is permanent. It CANNOT be undone. Continue?

No
Yes