Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove RuleHandler.js when expressions cover all features #169

Open
ivarne opened this issue Jan 27, 2023 · 6 comments
Open

Remove RuleHandler.js when expressions cover all features #169

ivarne opened this issue Jan 27, 2023 · 6 comments
Labels
kind/bug Something isn't working

Comments

@ivarne
Copy link
Member

ivarne commented Jan 27, 2023

Description of the bug

RuleHandler.js will soon be missing from the latest template in altinn studio.
I think this should be pre-emptively fixed by changing app-frontend-react to not crash when {org}/{app}/api/rulehandler/{id} responds with 404.

The nuget packages in app-lib-dotnet should probably also return 204 No Content instead of 404 Not Found so that it doesn't trash the browser error logs.

Update

This issue now tracks the removal of RuleHandler.js and associated files from the template, at some point in the future when dynamic expressions can implement most (or all) of the features used in RuleHandler.js.

@ivarne ivarne added the kind/bug Something isn't working label Jan 27, 2023
@olemartinorg
Copy link
Contributor

I agree. This should be an easy fix, and we're getting real close to the next release anyway. Adding this to the current sprint, and letting @bjosttveit and @Magnusrm know so they can chase me to it. 😉

@ivarne
Copy link
Member Author

ivarne commented Jan 27, 2023

I’ll make a PR for the status code change in backend. Missing this file is not an error anymore.

@ivarne ivarne changed the title RuleHandler.js is missing from the latest template in altinn studio RuleHandler.js will soon be missing from the latest template in altinn studio Jan 27, 2023
@olemartinorg
Copy link
Contributor

Turns out it's not as critical as we first thought. The file exists in the template, but was gone when an app was copied from altinn2convert.

Link to slack discussion:

@ivarne
Copy link
Member Author

ivarne commented Jan 27, 2023

Seems to me like the only thing required here is to make the backend reply with 204 No Content instead of 404 Not Found when RuleHandler.js is missing. Same solution seems to be used with RuleConfiguration.json that currently isn't in the template. (though it returns 200 Ok with an empty response). Not sure what status code makes most sense, but in Altinn/app-lib-dotnet#174 I decided to go with 204 No Content for both files.

@ivarne
Copy link
Member Author

ivarne commented Feb 1, 2023

With Altinn/app-lib-dotnet#174 merged (and soon released in the Altinn.App.Api nuget package), the only task left on this one is for @Altinn/team-altinnstudio to decide when to actually remove the file. Maybe this issue should be moved to altinn-studio?

@olemartinorg
Copy link
Contributor

Right. I believe removing the file as a default is an issue that should be handled by the apps and studio teams in unison (it's an apps thing, but needs studio support before we remove it). Sadly, expressions still don't support calculations, and there is not a 100% overlap in supported functionality, so we have no pressing plans to remove these files from our template AFAIK.

Btw, the template application is located in this repo: https://github.com/Altinn/app-template-dotnet

@olemartinorg olemartinorg changed the title RuleHandler.js will soon be missing from the latest template in altinn studio Remove RuleHandler.js when expressions cover all features Feb 8, 2023
@olemartinorg olemartinorg transferred this issue from Altinn/app-frontend-react Feb 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
Status: 📈 Todo
Development

No branches or pull requests

2 participants