isActiveAction should take query string into account

[amitaibu]

No description provided.

[amitaibu]

@mpscholten added tests so we have more confidence over the change! 🍻

When I try to add

isActiveController TestController: #{isActiveController @TestController}

I get:

[mpscholten]

The function can only be called from a view. To fix the test you can add a view data structure and render it to HSX

[amitaibu]

The function can only be called from a view

What indicates it? Is it the use of fromFrozenContext?

[mpscholten]

Yep exactly. The renderHtml does call Context.freeze before rendering the html https://github.com/digitallyinduced/ihp/blob/master/IHP/Controller/Render.hs#L60

[mpscholten]

Some explanation behind this is here https://github.com/digitallyinduced/ihp/blob/master/IHP/Controller/Context.hs#L14

[mpscholten]

Sorry, forgot to comment here, pushed a fix to https://github.com/amitaibu/ihp/pull/4/files Once merged the tests should be green 👍

[amitaibu]

Thanks. @mpscholten can you give a short explanation on how you knew what this fix is?

[mpscholten]

I dumped the response body of the the 500 Internal Server Error response. It showed that the error was happening here https://github.com/digitallyinduced/ihp/blob/master/IHP/Controller/Session.hs#L162 That error should never happen. I was first confused why the session was even used, because no code in the tests is doing anything with the session. I placed many error calls in my local IHP codebase to trace it back. Turns out that renderHtml calls initFlashMessages which then accesses the session.

Then I looked thorugh the test code and checked what could be wrong with the session setup. I figured out that we never called the session middleware. After that it was an easy fix, just wiring up the middleware so that the session is available 👍

[amitaibu]

I dumped the response body of the the 500 Internal Server Error response

How did you do it?

[mpscholten]

Changed the

assertTextExists body response = do
    response.simpleStatus `shouldBe` status200

function to this:

assertTextExists body response = do
    error (show response)
    response.simpleStatus `shouldBe` status200

[amitaibu]

@mpscholten tests are passing, but it shows the build as failing.

[mpscholten]

Thanks, looks good 👍

[s0kil]

I use this function often, and I expect isActiveAction to not take query string into account.

We should have isActiveAction as generic, and isActiveActionPath Or isActiveActionParams as strict param checking.

[amitaibu]

I use this function often, and I expect isActiveAction to not take query string into account.

How do you use it? How would it work on ShowPostAction (Id Post) if you don't pass the Post Id?

[s0kil]

With the example shown, the path would be /ShowPostAction?postId=UUID, But we can still match on /ShowPostAction.

[amitaibu]

I think for that we'd need an isActiveActionByBasePath (or some better but still explicit name).

[s0kil]

In a current project, I have a helper:

-- TODO: File An Issue In IHP, The Provided Func Is Broken
isActivePathOrSub :: _ => controller -> Bool
isActivePathOrSub route =
    let
        currentPath = Wai.rawPathInfo theRequest
    in
        currentPath `ByteString.isPrefixOf` cs (pathTo route)

That seems to work as well.