Skip to content

Conversation

@tuj
Copy link
Contributor

@tuj tuj commented Aug 31, 2025

Link to issue

#249

Description

Added online check.

Checklist

  • My code is covered by test cases.
  • My code passes our test (all our tests).
  • My code passes our static analysis suite.
  • My code passes our continuous integration process.

@tuj tuj added this to the 3.0.0 milestone Aug 31, 2025
@tuj tuj requested a review from turegjorup August 31, 2025 08:14
@tuj tuj self-assigned this Aug 31, 2025
@tuj tuj added the enhancement New feature or request label Aug 31, 2025
@tuj tuj mentioned this pull request Aug 31, 2025
35 tasks
Copy link
Contributor

@turegjorup turegjorup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current URL is https://os2display.example.com/client/online-check/ (note /client). This will change that to https://os2display.example.com/online-check/ without making a redirect which is an unnecessary breaking change.

Please move the files to /public/client/online-check so that 1) we don't change the url, 2) it scopes the files to the client

CHANGELOG.md Outdated
* Upgraded redux-toolkit and how api slices are generated.
* Fixed redux-toolkit cache handling.
* Add Taskfile
* Added online-check to public.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Added online-check to public.
* Added (screen) client online-check to public.

@tuj
Copy link
Contributor Author

tuj commented Sep 1, 2025

Atm /client/online-check/ is not served as static files. The nginx config should be modified.

README.md should also have a section added with a description of the Online Check setup.

@turegjorup
Copy link
Contributor

Have updated the nginx config so /client/online-check is served correctly.

Question. Why is this necessary?

# /config/routes.yaml

client:
    path: /client{subroutes}
    requirements:
        # https://symfony.com/doc/current/routing.html#slash-characters-in-route-parameters
        subroutes: '.*'
    methods: ['GET']
    controller: App\Controller\Client\ClientController

What are the subroutes used for?

@tuj
Copy link
Contributor Author

tuj commented Sep 1, 2025

Have updated the nginx config so /client/online-check is served correctly.

Question. Why is this necessary?

# /config/routes.yaml

client:
    path: /client{subroutes}
    requirements:
        # https://symfony.com/doc/current/routing.html#slash-characters-in-route-parameters
        subroutes: '.*'
    methods: ['GET']
    controller: App\Controller\Client\ClientController

What are the subroutes used for?

For the Admin and Template routes they are used for passing all the path handling to the react app. This is not actually needed for the client, and should be removed.

@turegjorup
Copy link
Contributor

For the Admin and Template routes they are used for passing all the path handling to the react app. This is not actually needed for the client, and should be removed.

Good point. Didn't think about client side routing in the admin app. I think we can leave it in for consistency.

@tuj tuj requested a review from turegjorup September 1, 2025 13:42
@tuj tuj merged commit fb32d73 into release/3.0.0 Sep 1, 2025
17 checks passed
@tuj tuj deleted the feature/online-check branch September 1, 2025 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants