Good PR

Instructions for QA

Preconditions

Software to be installed in the machine:
- Latest NodeJS
- Web Browser, evergreen preffered: Google Chrome, Firefox, Brave, Edge
- Git

Setup

Clone the repo

Open a new command line prompt in your local machine and run git clone https://github.com/asorian0/firstblood-exercise.git. Since it is private repo it would be asking you for credentials. Use your github credentials, or sign-up if you haven't.

Install dependencies

Once the repo has been cloned, go into the folder executing cd firstblood-exercise and run npm install.

Deploy the application

Deploy the application locally running npm start. You'll see the message ** Angular Live Development Server is listening on localhost:4200, open your browser on http://localhost:4200/ ** once it is ready.

Manual test

Open the web browser and navigate to http://localhost:4200. After loading it should go to http://localhost:4200/sign-up. This is the form view. What to test:

All fields in the form are required
Email validates against regular expression expecting valid email like [email protected], so strings like user.host.com or user@host must lead to field marked in red
Phone number country selection opens on click, it allows search by typing country name (in english) as well as scrolling. Option list displays flag, country name and prefix. Once selected, it displays flag and prefix and focus the number input. This input allows only numbers, and it is updated while typing with the preferred format based on country selection. When number is not valid, but considered possible, the format disappears. That possible status comes from the fact that number length is good enough to be valid but seems longer than the perfect match to be valid, so it is considered possible. This is a feature from the library being used under the hood
Phone number tries to find the user country checking navigator.languages
Password fields must match
Retype password validation is triggered when password value changes. The use case is like: type test1234 in retype password field, it will be marked as red. Then type test1234 in password field and retype password field will become valid
Password fields allows to show the actual string by clicking on the eye icon, as well as get back to display the hidden string clicking it again
While typing the password there is a bar showing strenght for the string being typed right below the field
There are no specific limitations on the password string since this is out the scope
If form is not valid, by clicking on submit button any not valid field will be displayed in red
Once the form is valid, by clicking on submit button user is redirected to http://localhost:4200/home where user account details are displayed

Structure

Usually I prefer to work with the recommendation from Angular dev team, so for we'd be coming up with a tree like this:

- app
  - app.component.*
  - app.module*
  - shared
    - services
      - user.service*
    - models (models that are shared across different modules)
    - pipes 
    - components
      - nav-bar
      - footer
      - footer-only-frame (this component should be renamed to something like "frame" or "main-wrapper")
      - full-frame (clearly this component and "footer-only-frame" can be merged and handle that main view from the same component)
      - phone-input
    - material.module.ts (and any other barrel module)
  - auth (feature module to handle login, sign up, reset password etc - views for not logged in users)
    - sign-up-form
    - sign-up-view
    - shared (folder to push services, models, pipes etc only related to this module)
    - auth.module.ts
    - auth-routing.module.ts
  - home (feature module to handle home user)
    - user-dashboard
    - user-home
    - shared (folder to push services, models, pipes etc only related to this module)
    - home.module.ts
    - home-routing.module.ts

That way the code is less coupled, and a module can be easily moved/changed by other one without lot of effort.

Some extra considerations

Password needs to be salted before posting it to the backend
Currently there is no good Unit Tests coverage (actually I didn't add any UT there cause I understood it is out of the scope)
Unit tests are running over full Chrome browser, it would be faster to run them against headless browser
Changed primary color by accent in form fields to make their labels more readable
Adding bootstrap to avoid writing manually paddings, margins. Noting this cause I could be using fxFlex from Material Layout instead, but Bootstrap allows the same plus those pretty convenient classes. It also applies border-box: * by default. Less written code, less code to maintain ;)
Kept E2E approach in order to reuse the current code, but usually I would be using Cucumber + Protractor cause it allows to save a lot of lines of code
Some suites should be created in E2E so we could easily run smoke, feature-focused and full integration tests (for example)
Usually would created a new file to store the .form-element styles and imported them into styles.css in order to keep a good split of concerns in the style files, but given that it was just a few lines I didn't took that approach
I've updated to Angular 10 and its dependencies
I didn't added all E2E cases pointed to QA cause I already spent a lot of time on this, but they should be added there
Didn't added Window token cause Unit Tests are out of the scope, but given that Storage API and Navigator API are being used we should inject it in those services using those APIs to handle UTs in a better way

Time spent

8 hours, since I had to write the phone-input component from scratch

miohtama/good-pr.md