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

[frontend] Fix quick subcription button(#5963) #6002

Merged
merged 4 commits into from
Feb 20, 2024
Merged

Conversation

CelineSebe
Copy link
Member

Proposed changes

  • [frontend] Take out usePreloadedQuery function
  • [frontend] Add useEffect to update existingInstanceTriggersEdges

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...

@CelineSebe CelineSebe added the filigran team use to identify PR from the Filigran team label Feb 16, 2024
@CelineSebe CelineSebe self-assigned this Feb 16, 2024
@CelineSebe CelineSebe linked an issue Feb 16, 2024 that may be closed by this pull request
Copy link
Member

@frapuks frapuks left a comment

Choose a reason for hiding this comment

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

it works

Copy link

codecov bot commented Feb 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (784d9ba) 65.44% compared to head (68d8605) 67.64%.
Report is 55 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6002      +/-   ##
==========================================
+ Coverage   65.44%   67.64%   +2.19%     
==========================================
  Files         539      539              
  Lines       63055    63568     +513     
  Branches     5052     6540    +1488     
==========================================
+ Hits        41269    43003    +1734     
+ Misses      21786    20565    -1221     

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

@CelineSebe CelineSebe changed the title [frontend]Quick subcription button is working properly(#5963) [frontend] Fix quick subcription button(#5963) Feb 16, 2024
Copy link
Member

@Kedae Kedae left a comment

Choose a reason for hiding this comment

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

We need to check with @Archidoit what was the goal of the previous code

const handleOpen = () => {
searchInstanceTriggers();
Copy link
Member

@Archidoit Archidoit Feb 16, 2024

Choose a reason for hiding this comment

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

@Kedae this was to update the instance triggers list in case the user had deleted/updated one of triggers via the quick subscription edition panel

Copy link
Member

@Archidoit Archidoit Feb 16, 2024

Choose a reason for hiding this comment

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

but seems to work without it now that the 'pagination keys' are the same as Céline told me , strange

Copy link
Member

@Kedae Kedae left a comment

Choose a reason for hiding this comment

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

I read wrong Cathia's message, we need to discuss it

@CelineSebe CelineSebe marked this pull request as draft February 20, 2024 09:57
@CelineSebe CelineSebe marked this pull request as ready for review February 20, 2024 14:42
@CelineSebe CelineSebe merged commit e1c2692 into master Feb 20, 2024
8 checks passed
@SamuelHassine SamuelHassine deleted the issue/5963 branch February 23, 2024 00:24
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.

Quick subcription button is not working properly
5 participants