Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[backend/frontend] adding decay chart in indicator lifecycle overview (#2859) #5639

Merged
merged 9 commits into from
Jan 30, 2024

Conversation

aHenryJard
Copy link
Member

@aHenryJard aHenryJard commented Jan 22, 2024

Proposed changes

Chunck 3 of decay topic: add visual graph in Indicator overview.

  • graph data is computed on the backend and fetch when opening the Indicator detail.
  • the point on the graph is compose of all integer score values + time
  • all color are taken from the palette, if you think we need some new one feel free to tell me
  • the DecayChart.tsx is not generic yet, this will be done with next chunck (settings screen)
  • No translation for now, all Decay translation will be done at the last chunck (when texts are stable)

Dark mode:

Capture d'écran 2024-01-29 182212

Light mode:

Capture d'écran 2024-01-29 182827

Related issues

Checklist

  • I consider the submitted work as finished
  • I tested the code for its functionality
  • I wrote test cases for the relevant uses case
  • I added/update the relevant documentation (either on github or on notion)
  • Where necessary I refactored code to improve the overall quality

Further comments

If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...

@richard-julien richard-julien added the filigran team use to identify PR from the Filigran team label Jan 22, 2024
Copy link

codecov bot commented Jan 25, 2024

Codecov Report

Attention: 16 lines in your changes are missing coverage. Please review.

Comparison is base (642071c) 66.15% compared to head (bd48950) 66.43%.
Report is 12 commits behind head on master.

Files Patch % Lines
...-graphql/src/modules/indicator/indicator-domain.ts 7.69% 12 Missing ⚠️
...raphql/src/modules/indicator/decay-chart-domain.ts 91.83% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5639      +/-   ##
==========================================
+ Coverage   66.15%   66.43%   +0.28%     
==========================================
  Files         513      514       +1     
  Lines       60788    60869      +81     
  Branches     4447     4614     +167     
==========================================
+ Hits        40214    40439     +225     
+ Misses      20574    20430     -144     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@aHenryJard aHenryJard changed the title [backend/frontend] adding decay chart in indicator lifecycle overview… [backend/frontend] adding decay chart in indicator lifecycle overview (#2859) Jan 26, 2024
@aHenryJard aHenryJard marked this pull request as ready for review January 26, 2024 17:25
Copy link
Member

@marieflorescontact marieflorescontact left a comment

Choose a reason for hiding this comment

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

Not a big deal but on the horizontal line we can see the creation date but not the revoke date, maybe it is done on purpose ?
otherwise it works fine 👌

@lndrtrbn lndrtrbn self-requested a review January 30, 2024 08:16
Copy link
Member

@lndrtrbn lndrtrbn left a comment

Choose a reason for hiding this comment

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

Code review done, only one remark repeated at different part of the code. I'll test locally now

@SouadHadjiat SouadHadjiat merged commit d0736ee into master Jan 30, 2024
8 checks passed
@SouadHadjiat SouadHadjiat deleted the issue/2859-chunk3-graph branch January 30, 2024 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
filigran team use to identify PR from the Filigran team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants