Code Refactoring - Part 1 - Scope Out Needed Changes and Improvements

[saidghamra]

In order for the UI team to quickly add new features and fix bugs, we decided to overhaul the code base for the UI. This issue is part 1 of the multi part process of refactoring the code base.

In this issue, Waleed will go through the code base on his own, come up with suggestions and improvements the UI team can make and then leave comments with his findings. We will then meet this upcoming Saturday (May 23, 2021), go over Marc and Waleeds findings, decide the changes that need to be made and start the refactoring process.

[waleedza99]

SuggestionsFromWaleed.txt

[MarcDAFrame]

• What is src\types\global.d.ts ? is it being used anywhere ? if not then we should probably get rid of it...

  • typescript file

• src\assets\favicon.png is an invalid file, has no image to display as a logo.

  • we will put our logo here once we have one

• Is src\app\styles\colors.scss being used ? if not then we should get rid of this styles directory entirely...

  • we need to unify the way we style

• Anywhere where there is loading happening we should display a loading screen.

• We are missing comments nearly in all the code we wrote for UI...

• Ask Said about the extension which automatically cleans up the spacing and formatting of the code.

• Theres very little code in app/stores/UIStore.ts, so maybe we can move the code within it elsewhere or maybe this store can be later utilized for more things than just the delete dialog and in that case we can keep this store.

  • keep it, more stuff will arrive

• In app\stores\UserStore.ts, the constructor requires a parameter of 'rootStore' but then the store doesn't use that parameter anywhere in the code... Maybe we should remove 'rootStore' from the constructor.

  • rootStore can be removed
  • best to have inter-store communication done with events

• We need to remove all existing commented code because it creates confusion and a mess.

  • agreed
  • we'll go through our own files and remove them as we need to

• The file: app\models\Base\index.ts looks like it contains a lot of repetitive code, very messy looking, so it will probably require hard refactoring...

  • Marc's working on this

• We should change the name of "Base" directory in app\models\Base to something like "API-Model" or maybe just plain "API" because this model is all about api calls and handling responses. This would mean that there would be changes required in app\models\Base\index.ts as anywhere 'BaseModel' is used we would have to change that to 'APIModel'. Maybe we should not reuse the word "Model" in the names if the files are already under the Model directory...

  • model represents more than just the API
    • APIs
    • forms
    • Client State

• app\models\User\index.ts, this contains for what it seems like regular user(s) and then workspace user(s), so if we want to switch to the following file structurce:

• In src\models\Workspaces\Projects\Lanes\index.ts we have an interface called 'laneData_i', are we sure that this interface needs to be this long? can we shorten it ? if we can't then we should probably move it to a separate file because its longer than the code thats in the file for the model. This isn't the only model with a giant interface i.e. theres another long one in src\models\Workspaces\Projects\Lanes\Tasks\index.ts

  • this is in the @agoracloud/common package, no longer have to copy and paste

• Why are some folder names capitalized but some are not (i.e. models\Workspaces) ?

  • make all folders lowercase

• Should the folder: src\models\Workspaces be renamed to src\models\Workspace because A workspace has Deployements, Metrics, etc.

  • make model names singular: Workspaces -> workspaces

• We should replace app\forms\validators.ts with the shared package validators

  • agreed

• src\app\forms\Permissions\roles.tsx is empty... we should remove it then no ?

  • roles feature will be added after the refactor

• I feel like all the random solutions which didn't belong in any other directory were arbitrarily placed inside app\containers\Home, everything in this Home folder just looks random

  • Home should be named workspace
  • the nesting of the folders should align with the url format
    • i.e. /w/<WID> -> /containers/workspace/index.tsx and /w/<WID>/projects -> /containers/workspace/projects/index.tsx

• looks like app/containers/Home/UpdateWorkspace/index.tsx has a lot of repetitive code...

• Is src\app\containers\Home\style.scss being used anywhere ? if not we should probably remove this file...

• Ask Marc if its possible to use the Table component in app\components\Table for Projects page because it looks resuable

  • yep

• Should the app\components\Redirects and app\components\Guards\RouteGuards all go under one directory named "Routing" or "Routes" or something...

  • good idea
  • get rid of folder called guards, keep RouteGuard child
  • move Redirects into RouteGuards

• looks like app\components\Forms\Workspace\index.ts has a lot of repetitive code...

[waleedza99]

SuggestionsFromWaleed.txt