Footer - Githubissues

bjosttveit commented 1 year ago

Description

Removed the proposed grid-solution as I think it could be too complicated for a user friendly configuration in studio. Currently just a simple flexbox. Cypress tests depend on https://github.com/Altinn/app-lib-dotnet/pull/183 being released and frontend-test updating nugets.

Related Issue(s)

closes #538
closes #110

Verification

Manual testing
- [x] I have tested these changes manually
- [ ] Creator of the original issue (or service owner) has been contacted for manual testing
- [ ] No testing done/necessary
Automated tests
- [ ] Unit test(s) have been added
- [x] Cypress E2E test(s) have been added
- [ ] No automatic tests are needed here
- [ ] I want someone to help me make some tests
User documentation @ altinn-studio-docs
- [x] Has been updated
- https://github.com/Altinn/altinn-studio-docs/pull/836
- [ ] No changes/updates needed
Changes/additions to component properties
- [ ] Changes are reflected in both src/layout/layout.d.ts and layout.schema.v1.json, and these are all backwards-compatible
- [x] No changes made
Support in Altinn Studio
- [x] Issue(s) created for support in Studio
- https://github.com/Altinn/altinn-studio/issues/9810
- [ ] This change/feature does not require any changes to Altinn Studio
Sprint board
- [x] The original issue (or this PR itself) has been added to the Team Apps project and to the current sprint board

ivarne commented 1 year ago

Sorry for not noticing this earlier, but I somehow feel this is very related to the issues about customizing the header.

Why did you go with a custom footer.json file instead of making this just a footer property of applicationsettings.json or ul/../Settings.json?

Another consequence of the current setup is that we need yet another http call to show an app (making loading even slower), and for all older apps it will fill our logs with 404 Not Found {org}/{app}/api/footer, both in console and in application insights (where you might be able to configure an ignore rule.)

bjosttveit commented 1 year ago

Those issues are indeed related, and are something we also want to allow customization of.

I suppose the reason we did not want to define it in applicationMetadata.json is that it would not be very flexible in case we wanted to allow different footers depending on the process step in the future. It cannot really be put into Settings.json since the footer needs to be available outside of the process, like instance selection. Yes, it would cause 404 in older apps as of now. 😕

bjosttveit commented 1 year ago

We have discussed this again today, and we have decided to keep it in a separate file with the current setup for now. And then at a later point we can combine multiple files into a single request were appropriate to reduce the number of requests when loading the form.

ivarne commented 1 year ago

OK: Looking forward to Application Insights being full of 404 errors looking for a footer.

olemartinorg commented 1 year ago

OK: Looking forward to Application Insights being full of 404 errors looking for a footer.

@ivarne Same thing when looking for api/v1/featureset in #899 soon. We'll discuss this on our end, to try to find a solution. IMO, 404 is barely an "error" (reminder that 4xx means "you did something wrong" while 5xx is "I did something wrong", and if lots of 4xx responses on an app trigger all the alarm bells it would be very easy for any malicious actor to make those alarm bells go off).

ivarne commented 1 year ago

@olemartinorg Yes, but api/v1/featureset is one endpoint that will reduce noise in the future. api/v1/footer is yet another minor feature that shouldn't need to probe for the existence of an api.

bjosttveit commented 1 year ago

Nice, I assumed I needed to update frontend-test with a footer and updated nugets to test with cypress. If it is possible to mock I can do it before merging 👍