RFC 174 - Consolidate Frontend Apps #174
Conversation
leenagupte
changed the title
leenagupte
changed the title
leenagupte
changed the title
leenagupte
changed the title
leenagupte
force-pushed
the
consolidate-frontend-apps
branch
from
8ac7a71 to
c2242b5
Compare
leenagupte
force-pushed
the
consolidate-frontend-apps
branch
3 times, most recently
from
53a5e8a to
e79b94b
Compare
leenagupte
force-pushed
the
consolidate-frontend-apps
branch
from
b69c306 to
2c362b0
Compare
leenagupte
force-pushed
the
consolidate-frontend-apps
branch
from
2c362b0 to
db1bbd3
Compare
Co-authored-by: Andy Sellick <[email protected]> Co-authored-by: Max Harden <[email protected]> Co-authored-by: Keith Lawrence <[email protected]>
leenagupte
force-pushed
the
consolidate-frontend-apps
branch
from
db1bbd3 to
3030f4e
Compare
leenagupte
changed the title
| We propose using [frontend] as the host app. This is because: | ||
|
|
||
| * `frontend` contains some of the more complicated document types like licence_transaction and local_transaction, that have multiple pages and interactive elements, which would be harder to consolidate into another app | ||
| * `frontend` has a one-controller per document type layout, which is preferable for document types with multiple pages as it follows [rails conventions for controllers] to do the orchestration. |
There was a problem hiding this comment.
The controller-per-document-type approach does help in a number of ways. In order to do this though, (somewhat unusually for GOV.UK apps) we use a routing constraint based on document type. This retrieves the content item from content store as part of the routing of a request and checks the document type to determine which controller should handle it.
The content item is cached in the request object, so should only be retrieved once - even though it may be used multiple times in route resolution. Additionally, the controller for document types that use content items also retrieves the content item from the request object.
I don't think this is a problem, and might well be a useful approach when introducing new types to the app, however it is probably not well known so should probably be socialised at least. It would probably be a Bad Thing:tm: if our single frontend app made duplicate requests to our APIs!
There was a problem hiding this comment.
Oh that's interesting point. Thanks Simon! We were concerned that we'd be retrieving the content-item twice. But you're right, getting the content item from the request object rather than an api call is a slightly odd pattern.
There was a problem hiding this comment.
Do bear in mind that the frontend approach of preloading the content item makes the Rails log a bit bonkers, the logging all happens in the controller and as the content store call happens before then you can have what looks like crazy fast responses when Content Store is very slow.
I'm not sure the best thing to do about it but it will make any content store incidents harder to diagnose while proliferated further.
There was a problem hiding this comment.
Oh, that's a shame, and also good to know. 😬
There was a problem hiding this comment.
Another advantage to the one-controller-per-document-type approach is it makes the metrics easier to interpet.
Consider government-frontend which just has content_item#show vs. frontend where you can see what's going on in each controller action. We could add more granularity to the metrics in government-frontend, but the advantage of using different rails controllers is that we don't have to think about it - it works the way the metrics library expects.
| 4. [finder-frontend] | ||
| 5. [frontend] | ||
| 6. [government-frontend] | ||
| 7. [smart-answers] |
There was a problem hiding this comment.
Although technically a rendering app, Smart Answers works very differently to other rendering apps (it doesn't render items from the Content Store, for example), and I'm not sure it's code structure would fit into the suggested format very well.
I'm sure lots of folks are aware of this, but I just want to call this out as something that will probably need special attention.
There was a problem hiding this comment.
Oh yes, we included it for completeness. My thoughts about smart-answers are that it probably needs to be deconstructed into 3 apps. The templates for rendering, a publishing app for the content, and a gem / engine / app for the business rules / logic that can be imported into both the publishing and rendering apps.
There was a problem hiding this comment.
Don't discount merging it into a single app with both rendering and an admin area, would be way simpler to build than multiple apps.
There was a problem hiding this comment.
I think whatever the solution to smartanswers, it will merit its own RFC.
For this RFC, smartanswers was included for completeness. Realistically I think we could get to a place where most of the rendering is done in frontend and frontend does the redirects to smartanswers routes removing the need for router and router-api.
There was a problem hiding this comment.
Don't discount merging it into a single app with both rendering and an admin area, would be way simpler to build than multiple apps.
I feel like we've recently seen a bunch of work in Whitehall to get away from the publishing/rendering in a single app paradigm, how would a single admin + rendering smart answers differ from that? We have things that are intermediate to that (eg places-manager, where we have an API consumed by the frontend and the admin console for that API in one app). A Smart-Answers manager, maybe, and then smart answers would get published as normal artifacts and attached to an API object? (in the same way a Place is an item in Publisher that is published as normal, but attached to a service in Places Manager which provides the backing data)
There was a problem hiding this comment.
I'd not use Whitehall as a gold standard in this area because it spent many years being an app that wasn't rendering about 90% of the content and just 10%. I think if it had been the other way around and Whitehall was still rendering 90% of the content we'd have probably just migrated the other way to get it back to 100%. The debts of that system were always the inconsistencies and sheer complexity.
I think we shouldn't form a holy rule that publishing and rendering must be separate. Obviously we've invested a lot in having an architecture that supports separation and if we have basic content that goes into Publishing API and then a rendering app that just pulls data from content store same way as other apps then you'd be best continuing that pattern. But for something like Smart Answers where we've got complex modelling system, ability to define custom logic and a relatively small number of domain concepts, surely the overhead of having multiple apps to keep in sync would be a burden more than a benefit? Also seems unlikely that you'd want separate teams to have frontend and backend given the heavy coupling.
Obviously this is all rather academic at this point given the many years of desire for a Smart Answer management system and lack of solutions mind 😄
There was a problem hiding this comment.
Well, it wouldn't end up different teams if it were like Places (there the published item is quite light, and the backing service API management is the main thing, managed by a single team). But sure. We're probably not going to solve the problem of Smart Answers in this comment thread ;)
There was a problem hiding this comment.
A reason why I think we could break up smart-answers is because we already have something similar for simple-smart-answers. Yes, it's a much simpler wizard, but it is configured in a publishing app, but the rendering is separate.
I think issue with regular smart-answers has always been the amount of developer toil to update the rates and content that really should be editable by content designers. So, I do think it needs to be torn up and re-thought, especially as the developer numbers aren't what they used to be.
I wouldn't ever take the conversation about smart-answers off the table. The scope of it is too large to be covered by this RFC, but it does do frontend rendering and needs to be considered with the other apps.
| * The test suite will take longer due to more tests. | ||
| * could potentially parallelise in CI |
There was a problem hiding this comment.
A shorter term solution could be pay for GitHub Action runners with more CPUs and let the testing framework handle the parallelisation. Could save time for now trying to figure out how to distribute across GitHub Action jobs.
| * Need to reconfigure Kubernetes deployment. We should be able to drop the total number of pods. | ||
| * optimise the number of pods needed in draft and production | ||
| * currently in production there are | ||
| * 4 pods for `frontend` | ||
| * 6 for `government-frontend` | ||
| * 4 for `collections` | ||
| * 9 for `finder-frontend` | ||
| * plus 2 or 3 per app for the others | ||
| * scaling up the pods for a single app will use more resources |
There was a problem hiding this comment.
I think the net resource usage would be roughly be the same. You'll have more "frontend" pods, but also you're removing pods for the other frontend apps - (also potentially router/router-api). Likely there will be higher memory usage per pod, but wouldn't have thought it'd be significant. Scaling the number of pods is a super simple configuration change that takes minutes.
There was a problem hiding this comment.
Essentially think this will be a non-issue - but defo worth keeping an eye on how the resources usage changes.
| Merging all of our apps together is not without difficulties and we acknowledge that a single app will present problems that our multiple apps do not. | ||
|
|
||
| * A Rails update for one large app might be more complex than spread across individual apps. | ||
| * The scaling of a single app might become more costly. |
There was a problem hiding this comment.
The cost of scaling these pods is likely be minimal vs our overall infrastructure spend.
|
Whilst i don't feel confident in commenting on the coding patterns as such, it does look like a good idea to combine everything as you describe. It will certainly contribute to savings and efficiencies (coding-wise, not necessarily in ££) in future in unforeseen ways. |
| * Consider and work on expected issues (see below) | ||
| * Work to merge remaining apps | ||
|
|
||
| ### Steps as part of a merge |
There was a problem hiding this comment.
You've flagged that the apps you're proposing to merge have inconsistent approaches to testing. And merging codebases as proposed here is one of the times we'd really benefit from having comprehensive test coverage. I wonder if there's a case for rewriting a candidate's tests in RSpec and ensuring good coverage prior to any merge in to frontend?
There was a problem hiding this comment.
You may be interested in this: alphagov/frontend#4090 (the PR that rewrites Frontend's test suite, since it's currently minitest). Of the candidate apps to merge in, all of them already have RSpec as their teset runner with the exceptions of government-frontend (the biggie) and smart-answers (which as other people have noted, probably isn't in scope for this realistically).
@DaveLee-GDS Those are really good points. At the moment the frontend apps are mostly owned by the web teams, so it's customary for the teams to ask the "owning" team for a review. When the frontend apps are merged, who "owns" it will become trickier. I think an option could be to have "contributor guidelines" where the conventions of the app are documented. For example, how to add a new document type, what sort of tests should be added and where etc. |
| * [Retire the candidate] once all routes have been removed | ||
|
|
||
| ### Expected issues | ||
| Merging all of our apps together is not without difficulties and we acknowledge that a single app will present problems that our multiple apps do not. |
There was a problem hiding this comment.
Will we see any of the benefits described from having less frontend apps, or will we only get the benefits when we have a single app?
I'm just looking at the number of different controllers in government frontend and feeling a bit scared about all that coming into frontend! We will have to be so ninja with reviews to prevent a wild west (recently wept whilst wrapping my head around all these services in collections that are doing basically the same thing)
Have we considered other ways of combining to get the number down to 2 (or 3)? Or do you not think that's a viable option?
There was a problem hiding this comment.
I think anything that reduces the number of apps even a little bit is a benefit as it reduces the maintenance burden, so 5 apps is better than 8 etc.
The benefit of the single app is being able to remove static and slimmer.
We will have to be quite strict in our reviews and making sure everything is properly namespaced, and in the agreed area of the code. I think that having contributor guidelines that details what is finally decided on here should make that a bit easier as there will be something to point to when doing a review.
When you say "other ways of combining" what do you mean?
|
|
||
| * Identify any issues with existing code | ||
| * size these issues | ||
| * either fix issues or record them as tech debt - we need to avoid trying to fix everything and concentrate on progressing consolidation |
There was a problem hiding this comment.
Candidate for should be fixed before moving over https://github.com/alphagov/government-frontend/tree/main/app/views/histories
There was a problem hiding this comment.
Ah, yes this content should be moved into a locale file. I think that can be "fixed" as part of the migration.
There was a problem hiding this comment.
Do we know the history of why the history pages aren't just normal content items? Is there a way we can remove them entirely from government-frontend and turn them into editable items?
There was a problem hiding this comment.
It would be nice to at least consider publishing the history pages as normal GOV.UK pages.
They've already got their own schema / document type, they're just missing a publishing app. (Note that it's a bit more complex than this in reality - the content items for these pages are only used by search at the moment - the frontend doesn't render them. Still though).
There was a problem hiding this comment.
Oh they're published by government-frontend at the moment 😬
It is a bit of a tangled mess, what came first? the template or the content item? 😬
As a first step the task that publishes the pages should be moved to a publishing app. They at least do for the most part seem to follow a pattern: breadcrumbs, contents list, image, govspeak.
There was a problem hiding this comment.
Awesome work here, It’s super exciting seeing work to improve our frontend architecture.
I’m very sold on the idea of combining together the applications that have the basic responsbility of rendering content store content however I’m not convinced it is a good idea to try have a single application for all of www.gov.uk. As it seems to me that it seems hard to justify the existence of both frontend and government-frontend is a good architecture decision, whereas it seems much harder to justify that merging all the email account management functionality of email-alert-frontend into frontend is going to offer much simplification benefits.
I also think that having a single app for www.gov.uk will be quite limiting, for example if this was already the architecture it’d be a bit of a nightmare for us to have to embed GOV.UK chat in frontend rather than a distinct app. Also if we had to have anything in a private repo we’d have to take the whole of www.gov.uk private.
I think it’d be prudent to reduce the scope of the RFC a bit, even merging the apps that seem very clear contenders is a project that’ll easily span north of a year. It’s really hard to imagine having the business runway that we could commit towards completing this project, just that we could expect to make gradual progress over time. For this reason I’m really pleased you’ve picked an existing app is a merge target as that reduces the risk. I can see that making frontend the dominant frontend app would offer the ability to explore retiring router.
I am quite skeptical of the ideas later in the RFC about engines, I can see how they (or packwerk) would be helpful in a really complex Rails app, but I don’t think we should have the goal of building an app so complex we need them. I think the fact that these might be useful may be an indication that we’re considering stuffing too much stuff into a single app.
| 4. [finder-frontend] | ||
| 5. [frontend] | ||
| 6. [government-frontend] | ||
| 7. [smart-answers] |
There was a problem hiding this comment.
Don't discount merging it into a single app with both rendering and an admin area, would be way simpler to build than multiple apps.
| * written in Ruby on Rails | ||
| * no database | ||
| * relies on data from API calls to populate pages | ||
| * calls the content API from [content-store] to get the basic page data |
There was a problem hiding this comment.
This information seems correct for: collections, finder-frontend, frontend and government frontend.
However I wouldn't include Email Alert Frontend, Feedback, Smart Answers and Static as having these behaviours (although Feedback is a bit weird). I also wouldn't expect them to be candidates to be merged into a single app as they have such distinct responsibilities (and would be tricky as many of them make use of sessions whereas the other apps are stateless)
There was a problem hiding this comment.
I think there's probably a couple of categories of apps, and we could do with being explicit about whether we're consolidating them all, or just the ones that match these similarity criteria.
It would still be a big win even if we just merged collections, finder-frontend, frontend and government frontend, but we wouldn't be able to get rid of router without an answer for all of them.
There was a problem hiding this comment.
The apps that are allowed cookies are:
- licensing
- email-alert-frontend
- frontend (for the sign-in (digital identity) callback)
Technically cookies are allowed / denied based on path, not app, so in theory merging them shouldn't make a massive difference.
https://github.com/alphagov/govuk-fastly/blob/main/modules/www/www.vcl.tftpl#L335-L341
There was a problem hiding this comment.
I think there's probably a couple of categories of apps, and we could do with being explicit about whether we're consolidating them all, or just the ones that match these similarity criteria.
I'll update this list. When we're talking about consolidating apps, we meant the apps that render pages or parts of pages to the user (general public), so not APIs.
|
|
||
| ## Proposal | ||
|
|
||
| We propose merging the apps into a single rendering app. This will solve some of the issues in this RFC and allow us to solve others in the future - for example, removing `static`. |
There was a problem hiding this comment.
I'd suggest limiting the proposal to merging just the ones that do the basic content item rendering (collections, frontend, government-frontend, finder-frontend) and not do the ones that have unambiguously distinct focus. Given merging those 4 is a big task alone we'll probably have to consider that we might not complete all of those in a reasonable time frame and cutting out the other ones will make the scope of this RFC much more achievable. It'd also reduce the risk of frontend becoming a monster too quicky.
| We propose using [frontend] as the host app. This is because: | ||
|
|
||
| * `frontend` contains some of the more complicated document types like licence_transaction and local_transaction, that have multiple pages and interactive elements, which would be harder to consolidate into another app | ||
| * `frontend` has a one-controller per document type layout, which is preferable for document types with multiple pages as it follows [rails conventions for controllers] to do the orchestration. |
There was a problem hiding this comment.
Do bear in mind that the frontend approach of preloading the content item makes the Rails log a bit bonkers, the logging all happens in the controller and as the content store call happens before then you can have what looks like crazy fast responses when Content Store is very slow.
I'm not sure the best thing to do about it but it will make any content store incidents harder to diagnose while proliferated further.
| [government-frontend] was also considered for the host app, but not chosen because it has a more restrictive presenter pattern that works well for single-page document types. Even though `government-frontend` has many document types and accounts for 50% of the frontend traffic, it would be easier to move the single-page document types from `government-frontend` to `frontend` than it would to try and force the document types in `frontend` into the presenter pattern. | ||
|
|
||
| ### Testing standardisation | ||
| We propose standardising around RSpec as the test suite for the frontend, ideally [modernising the tests], for example by switching to request tests rather than controller tests and system tests rather than feature tests. RSpec is the [preferred testing suite] on GOV.UK and is favoured by many developers because the tests and testing output are easier to read. |
|
|
||
| We aim to build our pages and assets to be as performant as possible, but repeated downloads invalidate some of that effort. This situation happens for any repeated components across the estate, where each app will re-deliver CSS and JavaScript to the user unnecessarily. | ||
|
|
||
| ### `router` / `router-api` are complicated and hard to maintain |
There was a problem hiding this comment.
I'm not sure this section is needed as the proposal doesn't go into work to resolve it
There was a problem hiding this comment.
I'll add a section. The thought is that if we can get down to say 2 apps, e.g. frontend and smartanswers then frontend could just redirect routes to the smartanswers removing the need for router and router-api.
There was a problem hiding this comment.
Ah so do you mean have frontend act as proxy similar to how router is?
I imagine the ideal would be if we had a more conventional proxy routing system where we used a path prefix for apps other than frontend, so /chat to chat, /email to email-alert-frontend, /smart-answers for Smart Answers then everything else to frontend. Then we could get rid of the need for the complex system. Only thing that seems to be problematic is the existing URLs and how divisive it'd be to change them (with Smart Answers seeming easily most divisive there)
There was a problem hiding this comment.
Agree, if we still have separate frontend apps. A path prefix would make it super easy to hand routing off to the load balancer. (I know the content folk like to have easy human readable URLs, but there needs to be compromise between the value that brings users vs engineering complexity here). Dealing with existing URLs would be tricky, but not impossible.
There was a problem hiding this comment.
As a stopgap, could Frontend (instead of Router) read from Router API and route to the other apps? What additional load would that put on the system? We're pretty heavily cached by the CDN, right? What would be the effect on the end rendering time if a rails app did the routing rather than the go app? I imagine it would be vastly slower, but does vastly slower actually equate to much in terms of what the end user sees?
There was a problem hiding this comment.
I don't know for sure but I guess it'd be a humongous load difference given that router deals with all the 404s and then government-frontend gets the most app traffic. Whether we can cope with it though seems like something we could work out, particularly if we could applying auto-scaling given the app is pretty much stateless so should scale well.
One thing I'm not clear on is whether Router conceals us a bit from some of the Ruby pains we have of only able to serve as many users as the number of threads we have per server, which can lead to requests in a backlog and eventually just aborted. But given Router already proxies to apps with that limitation I can't imagine it is that impactful.
What I'd probably suggest as a stop gap though is that rather than having Frontend start take Router API responsibilities you instead set Router to forward everything it doesn't know what to do with to Frontend, so Frontend can gradually take on handling error pages etc. You could then stop indexing in Router API for things to frontend so that the Router API database gradually gets smaller and smaller to be less impactful.
There was a problem hiding this comment.
I think we're already doing that with government-frontend, but I may be wrong. @theseanything is that right, or did I imagine that?
There was a problem hiding this comment.
I'd be in favour of dropping the section on router, as it seems quite likely that this RFC won't get us all the way to removing router / router-api - it's a big step in the right direction, but we'll have to do other things before we can get rid of router (and those other things probably grow the scope of this RFC too far).
There was a problem hiding this comment.
I think that rather than dropping the router section, we should add another that says how far we towards solving the problems with this RFC.
| * relies on data from API calls to populate pages | ||
| * calls the content API from [content-store] to get the basic page data | ||
|
|
||
| Some make further calls, for example to [search-api] to populate lists of search results. |
There was a problem hiding this comment.
Will we need to resolve search functionality in draft stack? I can't remember if apps like finder-frontend run in the draft stack where we don't have draft search (or if we just run them and use the live search)
There was a problem hiding this comment.
I think we'll need to resolve search functionality for running things locally and in heroku preview apps. I believe at moment you can only run new search via govuk-docker. This makes it harder to preview changes to the /search/all page.
|
Personally I'm not averse to a world of "many brains and one face" because my pain points are naturally around the rendering aspects. If we can find another way to a solution where we can run up a test instance that works for retracing user journeys end to end, can preserve states like logged in/accepted cookies, the header and footer know where the user is, it has one set of translations, and doesn't need to re-deliver assets repeatedly then I'm open to it. My end goal is probably consolidated rendering and I'm less bothered (but not unbothered) by the distributed dependency updates and so on. |
|
Hi @kevindew. Thanks for your comments. You made some really interesting points. I'm going to respond to them individually, so please bear with! |
| * written in Ruby on Rails | ||
| * no database | ||
| * relies on data from API calls to populate pages | ||
| * calls the content API from [content-store] to get the basic page data |
There was a problem hiding this comment.
I think there's probably a couple of categories of apps, and we could do with being explicit about whether we're consolidating them all, or just the ones that match these similarity criteria.
It would still be a big win even if we just merged collections, finder-frontend, frontend and government frontend, but we wouldn't be able to get rid of router without an answer for all of them.
|
|
||
| We aim to build our pages and assets to be as performant as possible, but repeated downloads invalidate some of that effort. This situation happens for any repeated components across the estate, where each app will re-deliver CSS and JavaScript to the user unnecessarily. | ||
|
|
||
| ### `router` / `router-api` are complicated and hard to maintain |
There was a problem hiding this comment.
I'd be in favour of dropping the section on router, as it seems quite likely that this RFC won't get us all the way to removing router / router-api - it's a big step in the right direction, but we'll have to do other things before we can get rid of router (and those other things probably grow the scope of this RFC too far).
| ### `static` and `slimmer` are complicated and inflexible | ||
| GOV.UK page layouts are rendered by `static` using the [slimmer] gem. This includes the header and footer common to all pages, even though the code for those elements comes from [govuk_publishing_components]. Having a single application would allow us to remove our dependency upon `static`. | ||
|
|
||
| `static` and `slimmer` are both quite complicated and not well understood among the current developer community. The details are beyond the scope of this document, but several specific problems can be identified. |
There was a problem hiding this comment.
I'm on the fence as to how much ambition to remove static we should have within the scope of this RFC...
I agree we're probably not going to merge everything to such a degree that we can naturally retire static / slimmer, at least not in the initial, foreseeable work on this RFC.
I like Kevin's idea of going static / slimmer free in the main frontend application, but we would need a solution to banners, which probably needs a separate discussion / maybe a separate RFC (I hadn't seen #118 before, but I like the idea of things like banners coming from content-store somehow). In some ways this actually makes matters worse though, because rather than the ~3 PRs you need now to update some things globally (change gem, release gem, dependabot static), you'd need 4 (change gem, release gem, dependabot static, dependabot frontend).
On the other hand, I can see the argument for treating the removal of static / slimmer as an entirely separate bit of work, just so we don't complicate this further.
| We propose using [frontend] as the host app. This is because: | ||
|
|
||
| * `frontend` contains some of the more complicated document types like licence_transaction and local_transaction, that have multiple pages and interactive elements, which would be harder to consolidate into another app | ||
| * `frontend` has a one-controller per document type layout, which is preferable for document types with multiple pages as it follows [rails conventions for controllers] to do the orchestration. |
There was a problem hiding this comment.
Another advantage to the one-controller-per-document-type approach is it makes the metrics easier to interpet.
Consider government-frontend which just has content_item#show vs. frontend where you can see what's going on in each controller action. We could add more granularity to the metrics in government-frontend, but the advantage of using different rails controllers is that we don't have to think about it - it works the way the metrics library expects.
| * modernise the tests | ||
| * audit `frontend` and make changes to the frontend directory structure to match “new layout” | ||
| * remove redundant code from frontend (non-blocking) | ||
| * remove publishing_api tasks from frontend (non-blocking) |
There was a problem hiding this comment.
Does this mean moving the bank-holidays config somewhere else? As I think the publishing_api tasks are basically there to shovel that data into publishing-api? Could do with saying where the functionality will move, or just dropping this line if it's not clear and it doesn't have to happen as part of this RFC.
There was a problem hiding this comment.
I think the bank holidays config should move to mainstream publisher. Is there a better app for it?
There was a problem hiding this comment.
Yeah, mainstream would be fine I reckon.
| * written in Ruby on Rails | ||
| * no database | ||
| * relies on data from API calls to populate pages | ||
| * calls the content API from [content-store] to get the basic page data |
There was a problem hiding this comment.
The apps that are allowed cookies are:
- licensing
- email-alert-frontend
- frontend (for the sign-in (digital identity) callback)
Technically cookies are allowed / denied based on path, not app, so in theory merging them shouldn't make a massive difference.
https://github.com/alphagov/govuk-fastly/blob/main/modules/www/www.vcl.tftpl#L335-L341
|
|
||
| * Identify any issues with existing code | ||
| * size these issues | ||
| * either fix issues or record them as tech debt - we need to avoid trying to fix everything and concentrate on progressing consolidation |
There was a problem hiding this comment.
It would be nice to at least consider publishing the history pages as normal GOV.UK pages.
They've already got their own schema / document type, they're just missing a publishing app. (Note that it's a bit more complex than this in reality - the content items for these pages are only used by search at the moment - the frontend doesn't render them. Still though).
| * A Rails update for one large app might be more complex than spread across individual apps. | ||
| * The scaling of a single app might become more costly. | ||
| * It won’t be possible to scale targeted parts of the stack (i.e. one of the existing apps) for expected increases in traffic, instead the whole site will need to be scaled. | ||
| * It’s only possible to test one branch of code in a test environment at once. If there’s only one repo, only one change can be tested at a time. |
There was a problem hiding this comment.
This is something we could fix with some infrastructure changes - we could make it so that more than one instance of the frontend was deployed in each test environment, and deploy different branches to different instances.
There was a problem hiding this comment.
That would be nice. That would also fix the problem we have at the moment with broken user journeys in the review apps.
|
|
||
| ### Convert apps to engines to mount in Frontend | ||
|
|
||
| If we wanted to keep the conceptually split functions of the apps but remove the rendering complexity of slimmer/static/router we could recreate the routes and business logic of the existing apps in Rails Engines that would be mounted inside the monolith. This option was rejected on the basis that it would not give us much code-separation advantage over simple namespace discipline within the monolith, and there would still be developer overhead in working with multiple repos. |
There was a problem hiding this comment.
We're trialling a new approach in whitehall, where we separate bits of code out into engines, but store them all in the same repo in lib/engines. There's only one example of this at the moment, the content object store.
This would allow us to use rails engines without the overhead of working with multiple repos.
It might be worth considering as a stepping stone, but I don't like it as a long term solution for merging frontends, because it retains the arbitrary split between "apps" (even if they're no longer actually apps).
There was a problem hiding this comment.
Yeah, if we're going to do this in frontend I'd want to be based on functionality rather than the historical app.
|
@kevindew thanks for your review. I'm going to respond to your big comment here, and then continue responding to the rest of them inline.
There's a lot of weirdness happening with the email pages as the journey can be split over multiple apps, making it harder to test. For example, if you want to sign up for emails about a taxon, you start in collections. Clicking on the "Get emails" link takes you to a page rendered by email-alert-frontend. If you're on the News and Communications finder and click on "Get emails" you're taken to a similar but different page rendered by finder-frontend, before continuing further into the email-alert-frontend journey. There's also some other areas of tech debt that would be nice to tackle. A lot of these issues would be easier to fix if the rendering was in one place. We could at least consolidate the templates the user is seeing and trying to smooth out the user journey from that perspective even if the routing in the backend is still all over the the place.
Unless I'm mistaken GOV.UK Chat purposefully doesn't look like the rest of GOV.UK? It's supposed to feel separate right? I also think that it doesn't have any of the same page furniture as a regular GOV.UK page? Is that right? If that's true it feels fine to keep Chat separate. If it was truly going to be embedded into user journeys in GOV.UK we'd have to properly think about how / where it fits in any case.
It's always felt weird that we do this. As in the content should be in draft, rather than github repos being made private.
Yes, as with every other GOV.UK migration it's unlikely that we'd be able to complete the consolidation. However we've decided to be upfront with that. The goal is to reduce the complexity of the frontend stack. So even consolidating 2-3 apps can be considered a win. Starting with an existing app means that we won't start be adding even more tech debt. It should just be a case of slowly reducing the debt. |
|
Thank you for all of your comments. We're going to close this RFC now and reopen two new ones in the coming weeks:
|
- in RFC-174 (#174 (comment)) increased cost of scaling was considered negligible.
Rendered version
Extended deadline for comments: 16/07/2024