# Principles

- Have UI layer (dumb components) decoupled from API
- Encapsulate logic to fetch on **page/smart** components. Delegate UI logic to dumb components
- Critical path has and should be tested
- Tests should improve our confidence in pushing code (atm they don't)
- Snapshot tests shouldn't be the **one size fits all tool**
- Thunks should be responsible for **all side effects**
- Reducers and action creators should **transform** API responses when needed
- Store should be *queryable* without much logic - when this is possible it means store is well structured - *think of the store as a database*

## Test priorities

UX - User experience
DX - Developer experience (influences speed, scalability, etc)

1. App doesn't break (UX)
2. Critical paths work (UX)
3. All user actions are either successful or given feedback to (UX)
4. I'm more confident changing/adding code because of this test (DX)
- Backwards compatibility
- Testing edge cases
- Test code shows how component is used
- Logic is tricky from the beggining

# ccp-dashboard-ui

Some of the principles listed are not always reflected on our code, in order to achieve maintainablity, we should progressively go towards them.

**Dependency layers**

Layers should depend on **the layer below**. Normally, if the dependency jumps one layer, that's raises a flag. Imagine, one dumb component (ServicesTable) depending on how the API sends his data, that's dangerous coupling. Following this allows tests to be contained and easy to write. And makes adding features to the API fine without messing with the UI, and vice versa.

-----

UI - Dumb components
__________
BUSINESS LOGIC - Pages components
__________
GETTERS/TRIGGERS - Selectors/Action creators
__________
STORE - Reducers/Action creators
__________
APIs - Thunks that do side effects (API requests, localStorage sets)
__________

# What to tackle first

## 1. Reducers/Store

1. Test Reducers/action creators - Reducers are and should always be **easy to test**. They're functional, with a desired input, a desired output should come.
- Critical for the application - if reducers/store is not working well, application **will not work**
- If reducers are well tested, there is flexibility to improve/change the store schema to ensure it is always healthy and makes sense (*store as a db principle*)
- Should be tested together with action creators and with real mocked json responses that are similar to what comes from the API (this way, if the API changes, it's just a matter of changing the mocked json and run the tests)
- If there is logic done on top of reducer data (imagine filtering) we should use the **selector pattern** - https://www.saltycrane.com/blog/2017/05/what-are-redux-selectors-why-use-them/

2. Change/improve/adapt store structure - The app and the API have grown, lots of things changed, some structures/store organization do not make sense anymore. After **phase 1**, we should be confident enough to change our store structure without breaking stuff
- Divide into smaller reducers (we already have complexity warning all over our code)
- Make sure we're calling things for the right names and that what works together is stored together
- Make it easier to query our store for any info we need

Examples:
- Get ingress with id `1`
- Get all k8s resources
- Get all StatefulSet resources
- Get StatefulSet with id `32`

This are examples of things that should be simple to query our store to get. If they're not simple, goal is not achieved (proper reducer structure and selectors enable this).

3. After an adapted and healthy store structured, *light caching* is easy because it becomes possible to not refetch some data when nothing changed, making navigation all around the application way faster and allowing us to do background fetching while the user is seeing something meaningful.

## 2. Pages

1. Get rid of `componentDidUpdate` and `componentDidMount` as much as possible - These two component hooks are normally a red flag that shows problems in component structure/logic. React's approach is mostly functional and these hooks are imperative. This makes it harder to reason about/extend the components that use it. There are usecases where they fit (or that might not be worth the effort to remove them) but most of them can be solved in a different, more functional way.
- Should be used to trigger actions that cannot be done in other way (Ex: thunks)
- Shouldn't be comparing props (this is a red flag)

2. Pages shouldn't be dealing with missing data. At the moment there are lots of checks to see if info is present (if organization, if user, if whatever). This should be handled in the less places possible. There are lots of components that shouldn't be aware of missing data, and that are.
- Selectors can be a place to deal with this
- Reducers can have defaults and be structured in a way to avoid this
- There can be wrapper components that handle with this kind of stuff (Example: Loading component that only renders when it as the essential data)

3. Page components shouldn't be dealing with actions that come out of other action (example below)

putApps(app, formType).then(msg => {
    if (msg.type === PUT_APPS_SUCCESS) {
        history.push(
            `/project/${project.id}/${
            resourceGroup.id
            }/components&integrations/${formType}`
        );
    }
});

The logic that happens inside `then` is quite dangerous, if the actions change we'll have to be changing a component, and that's a type of dangerous coupling.

4. Avoid re-requesting stuff that's already on store - Lots of requests are being done that are not needed.

*Example:*

Resource group > Networking - Fetches project and namespace
Resource group > Networking > External network - Fetches project and namespace

This can be handled with proper component structure (project and namespace are passed as props, or live inside a layer) or with a simple redux caching layer (storing the request timestamp, and automatically return if it is less than 5 minutes ago - example).

5. Test page components - After the steps listed above, page components will be way easier to test. If we test page components we can ensure that most of the **Test priorities** listed above will be met. After removing most of the hooks, reducing dependency from APIs, and breaking into smaller components, we will be able to create more **meaningfull** tests that are not just snapshots. Examples of things that should be tested:
- When user clicks button X, deleteAction should be called
- When user clicks on item Y, navigateToDetail action should be called with the desired input (well built url)
- Page shows loading when there is no info
- Page renders `PostgresForm` when the url is `postgres/create`
- Clicking `delete` button opens dialog to confirm
- Page renders with default data
- Forms are filled with data coming from the store
- Tests detect any missing import/variable/not well declared function

6. Get rid of plain string/hardcoded values - Most of the strings that are compared at the moment either are:
- A meaningful string on the backend (this should be turned into an enum)
- Related to another entities/data on the store, and should be handled dynamically
- A path to a route - Use url builders (we're getting there) as they add extra flexibility to change routes without hassle

7. Delete legacy code/reuse part of the logic - Specially with tables, there are lots of repeated logic. This can be handled in some ways, a few suggestions below:
- Creation of reusable functions, outside the components
- Creation of [hooks](https://reactjs.org/docs/hooks-intro.html) that are extracted and can be used in other places

This makes it easier to have all this logic tested. At the moment there are lots of repeated logic and tests that can be reduced to a few and grant that everything will still be tested. It would also make it easier to add new tests and build new features.

8. Use a local translations file - While we don't have translations, we could at least create a plain key -> value file that gets a translation from a code. We don't (yet) need to deal with multi language, but it will make it easier to deal with that in future if we already have a function that gets a translation out of a code. And this will not impact much of our work right now