Skip to content

Conversation

thegreatyamori
Copy link
Collaborator

No description provided.

@Angeluz-07 Angeluz-07 force-pushed the TT-155-consume-user-info branch from f402611 to 29e1b27 Compare March 19, 2021 16:59
@codecov
Copy link

codecov bot commented Mar 19, 2021

Codecov Report

Merging #650 (0949905) into master (13de42e) will increase coverage by 0.91%.
The diff coverage is 95.82%.

❗ Current head 0949905 differs from pull request most recent head 872952e. Consider uploading reports for the commit 872952e to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #650      +/-   ##
==========================================
+ Coverage   93.09%   94.00%   +0.91%     
==========================================
  Files          85      101      +16     
  Lines        1564     1836     +272     
  Branches      107      120      +13     
==========================================
+ Hits         1456     1726     +270     
- Misses         67       71       +4     
+ Partials       41       39       -2     
Impacted Files Coverage Δ
src/app/app-routing.module.ts 100.00% <ø> (ø)
...omponents/customer-list/customer-list.component.ts 92.10% <ø> (ø)
...time-entries-table/time-entries-table.component.ts 55.55% <ø> (ø)
...nents/time-range-form/time-range-form.component.ts 100.00% <ø> (ø)
src/app/modules/user/store/user.reducer.ts 62.50% <62.50%> (ø)
src/app/modules/user/services/user-info.service.ts 71.42% <71.42%> (ø)
src/app/modules/user/services/user.service.ts 75.00% <75.00%> (ø)
...dules/time-entries/pages/time-entries.component.ts 85.33% <82.60%> (+2.52%) ⬆️
...app/modules/login/services/azure.ad.b2c.service.ts 83.33% <91.66%> (+0.98%) ⬆️
...ponents/details-fields/details-fields.component.ts 93.54% <94.44%> (+6.36%) ⬆️
... and 47 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 80ccc1f...872952e. Read the comment docs.

Copy link

@kellycastrof kellycastrof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A question just to understand, why is there a UserEffect and a UsersEffect that come from the same module?

import { UserEffects as UsersEffects } from './modules/users/store/user.effects';
import { UserEffects } from './modules/user/store/user.effects';

@thegreatyamori
Copy link
Collaborator Author

thegreatyamori commented Mar 23, 2021

A question just to understand, why is there a UserEffect and a UsersEffect that come from the same module?

import { UserEffects as UsersEffects } from './modules/users/store/user.effects';
import { UserEffects } from './modules/user/store/user.effects';

Because there are two nrgx data stores.
The one we made is called UserEffect and it is used to query the logged user data.
The first one is UsersEffects and it is used to fetch all the registered users. With @Angeluz-07 we decided to add an alias to avoid having to modify the rest of the code since it did not correspond to us.
However, the name should be changed to UsersEffect to avoid future conflicts.
I suggest to change the name to UserListEffect

@scastillo-jp scastillo-jp merged commit 84e056c into master Mar 23, 2021
@scastillo-jp scastillo-jp deleted the TT-155-consume-user-info branch March 23, 2021 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants