Minor code refactors

[habbes]

Here are some minor, quick-to-fix issues with the codebase that I think should be refactored before we dive deep into the next milestone.

NB This task should not take more than a day to complete. If any of the issues on the list appears to take long, don't insist on working on it, let's discuss whether it can be rescheduled instead.

• Remove SMSService from the App interface
• Give SmsService and related type consistent, camel-case names (e.g. SmsService instead of SMSService, AtSmsService instead of AtSMSService
• The current SmsService interface seems tightly coupled to AT's API schema. I think the interface should be simpler and not have to expose the intricacies of AT's API. I believe the following interface should suffice for what we'll need it for:

interface SmsService {
  sendMessage(user: User, message: string): Promise
}

• The refreshData() action is called in many pages, but that data should be available to all pages after you log in. I think it should be moved to a centralized location, like the LoggedInStructure. I'm not sure why it was removed from there in the first place. I think it's okay to refresh the data in a different page if it needs some data after the initial refresh, in order to update its state or something. But let's be careful not to over-fetch. We might introduce bugs if we try to remove the refreshData calls in individual pages, so that's something we can do progressively, on the components we work on. I think we can also make it so that the components that needs the data render when the data is available, in order to avoid errors associated with rendering when data is not available.
• Let's remove the rate-limiting for now, or set it to a large number that's not likely to be reached by typical usage. I think we set it prematurely without knowing what to set it to. I think it's something we should concern ourselves with later, maybe when preparing the major release, and maybe at that point it would be more beneficial to use impose the limit per user ID, instead of per IP.
• Rename the LoggedInStructure to LoggedInLayout, or better yet DashboardLayout
• Rename the LoggedOutStructure to HomeLayout
• It might take a lot of time to change the current client validation code, so we can do that later, but please avoid using the same array-based system for new client-side validation code. I created an alternative that uses dictionaries (check validateNamedRules used in the sign up component. I also don't like this version that much, but it's more maintainable to work with field names than arbitrary array indexes. We can think of better validation in the future.
• There's a lot of code repetition, for example when you're resetting the fields in a component when it's been submitted or errored. You can create a clear method that sets the data to an object with clean fields, rather than creating that object everywhere. This makes it easier to add new fields to the form without forgetting to reset that field in some scenario you didn't know about
• The verifyGoogleIdToken is in the util/index.ts module, I think it should just be located in the user-service.ts, since that's the only place it's needed. Also, this method uses process.env.GOOGLE_CLIENT_ID instead of using config.googleClientId. The config.googleClientId should be passed to the args of the Users class, which should define the verifyGoogleIdToken method. Ideally, I think we should have separate auth providers, one auth provider for username/password and another for Google, and then each provider would take care of its own authentication logic instead of mixing a lot of if-statements in the login and createUser methods. But that's for another day
• We are abusing brace-less if statements in the codebase. I notice an abundance of cases like:

// if has no braces but else has braces
if (test)
  doOne();
else {
  doTwo();
  doThree();
}

// if with brace, else with no braces
if (test) {
  doOne();
  doOther();
}
else
 doTwo();

// if with no braces, but with other statements below it
if (test)
  doOne();
doOtherNotRelatedToIf();

I think this is just plain bad code that's likely to introduce bugs. Let's normalize using braces. Not using braces should be the exception. Here is the situtation where I think it's acceptable for us to have if without braces:

• When the if block has a single statement, does not have an else, and where the if block statement is written on the same line as the if condition, and there's an empty line below to make it clear the follow-up statements are not related to the if-condition:

if (test) doSomething();

doOtherNotRelatedtoIf();

So please correct misuse of the if-statements where you can find them. This is mostly prevalent in the client code (like when checking whether the user is set). When in doubt, use braces.