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

survey: move server-side all conditional and validation checks (?) #858

Open
tahini opened this issue Feb 14, 2025 · 7 comments
Open

survey: move server-side all conditional and validation checks (?) #858

tahini opened this issue Feb 14, 2025 · 7 comments
Labels
refactoring Requires refactoring

Comments

@tahini
Copy link
Contributor

tahini commented Feb 14, 2025

In a pass to cleanup and properly document redux actions (and what happens during their execution), it appears that there's a bit of gymnastic required because we have some values that are updated by the server (the serverFieldUpdate) and also validations that need to be done server side (like access code validation). Mechanisms were developed to support those and required a bit of changes client side, because at the time, we wanted to avoid breaking something that worked fine.

Now that we are breaking everything anyway, how about we move all interview side-effect code, like validations and conditional checks, to the server. Anyway, whenever these are called in the startUpdateInterview there's a call to the server, except in the case where there are no changes, like when submitting a section, but typically, a callback is called at that moment and that callback makes a server call.

What this implies is

  1. that a good part of what is currently in evolution-frontend/src/actions/Survey[Admin].ts and the files in the folder evolution-frontend/src/actions/utils would need to go to the backend. Widget preparation would be done there. The server would return a RuntimeInterviewAttributes object ready to be displayed.
  2. The widget config could be split (by webpack or the compiler), so that some fields go to frontend (labels), others to backend (conditional, validations) others to both (type, inputType, etc)
  3. Frontend is a lot dumber
  4. Backend has more work to do
  5. Would network latency be any more issue than it is currently? Probably not since there's one request anyway and pretty much the same code will need to run.
  6. Would the server resource requirement be increased?

Thoughts?

@tahini tahini added the refactoring Requires refactoring label Feb 14, 2025
@samuel-duhaime
Copy link
Collaborator

@tahini What is the main advantage of doing that?

@tahini
Copy link
Contributor Author

tahini commented Feb 14, 2025

Advantages:

  • No more distinctions and different code paths for client and server conditionals/side effects (aka serverFieldUpdate) and validations.
  • The server would be the only one to have the truth on the content of the interview: he receives updates to responses, updates the interview accordingly and sends back the changes to the client, who simply updates its own version of the interview, unlike now where the client interview changes, the server may change it too and chances are they may be discrepancies.
  • Client is more lightweight
  • Fwiw, we'd be a step closer to server-side rendering, that you, @samuel-duhaime, defended the other day ;-)
  • There may not be need for multiple back and forth when updating response, changing section, preloading section, getting the widgets of the new section. Currently, changing section will trigger 2 or 3 server updates, sometimes more.
  • We already depend on the network at each response change, so we don't really have an "offline" mode anyway.

Disadvantage:

  • More load and responsibilities on the server. Copilot mentions scalability as an advantage of the client-side code as it offloads the server. How does that affect us? Maybe we need some benchmarking before making a decision. Unless @greenscientist already has some metrics on that.
  • The interview process becomes more sensible to network latencies

@samuel-duhaime
Copy link
Collaborator

@tahini That's great. Also, this could help to save conditional results in the database. #725

@greenscientist
Copy link
Contributor

I don't see any travel survey in the world where scalability would be an issue. so far, even with out biggest survey, we only had a few users at the same time. I'm giving number out of "feeling" here, but unless we have more than 5 users per minute, which would be 300 respondant per hour et thus 4800 per day, I won't even think about scalability! Even then, that's quite low numbers.

A few extra thoughts:
We data and paradata should be on separate endpoints, which we could prioritise. Like sending paradata information should not block the client, and we can keep it in memory in the client and just update it once in a while. That would free bandwith for the flow for the actual data.
We can figure out ways to reduce latency and be less sensitive to network issues. We can lazy load all the question when we have time and have the server send the enable signal on the one hidden when we get the appropriate responses. Other ways: make sure we keep the conditional logic in a cache instead of having to hit the database every time. Both those can be late stage optimisation.

@tahini
Copy link
Contributor Author

tahini commented Feb 14, 2025

Here's another issue related to this #1, yes the very first issue in this repo!!

@tahini
Copy link
Contributor Author

tahini commented Feb 14, 2025

Most of the paradata we have right now relate to timing, so this will make it easier to differentiate what comes from client intervention and automatic side effects and their timing will come from the server (as currently). But indeed, when we do have client-side paradata, like mouse movement and clicks, then we can think on how to send it.

@tahini
Copy link
Contributor Author

tahini commented Feb 14, 2025

Here's a few more issues to consider if we move ahead with this. Not necessarily to fix them right away, but at least to think ahead: #776 #785 #814 #750 #742

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Requires refactoring
Projects
None yet
Development

No branches or pull requests

3 participants