-
Notifications
You must be signed in to change notification settings - Fork 43
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
🐛 notifications close all together bug #2118
base: main
Are you sure you want to change the base?
🐛 notifications close all together bug #2118
Conversation
01b7f08
to
dd06378
Compare
Signed-off-by: TalyaNaima <[email protected]>
Signed-off-by: TalyaNeima <[email protected]> Signed-off-by: TalyaNaima <[email protected]>
Signed-off-by: TalyaNaima <[email protected]> Signed-off-by: TalyaNaima <[email protected]>
dd06378
to
a771b1d
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2118 +/- ##
==========================================
+ Coverage 39.20% 41.97% +2.77%
==========================================
Files 146 175 +29
Lines 4857 5629 +772
Branches 1164 1341 +177
==========================================
+ Hits 1904 2363 +459
- Misses 2939 3250 +311
- Partials 14 16 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
c88e295
to
7a473d6
Compare
Signed-off-by: TalyaNaima <[email protected]>
7a473d6
to
6658065
Compare
@TalyaNaima -- Do you intend to move this PR out of draft? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please correct the title/description - it should provide a brief summary of the fix.
It would be also nice to attach some screenshots.
@@ -507,7 +507,8 @@ | |||
"success": { | |||
"saveWhat": "{{type}} {{what}} was successfully saved.", | |||
"save": "{{type}} was successfully saved.", | |||
"applicationDeleted": "{{appIDCount}} application(s) successfully deleted.", | |||
"applicationDeleted": "{{appName}} application(s) successfully deleted.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a native speaker but I would suggest Application {{appName}} successfully deleted.
@@ -345,6 +353,7 @@ export const ApplicationsTable: React.FC = () => { | |||
businessService: app.businessService?.name || "", | |||
tags: app.tags?.length || 0, | |||
effort: app.effort || 0, | |||
analysis: app.tasks.currentAnalyzer?.state || 0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this change related to the fix? If no then please revert.
@@ -7,7 +7,7 @@ export interface ApplicationAnalysisStatusProps { | |||
state: TaskState; | |||
} | |||
|
|||
const taskStateToAnalyze: Map<TaskState, IconedStatusPreset> = new Map([ | |||
export const taskStateToAnalyze: Map<TaskState, IconedStatusPreset> = new Map([ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this change related to the fix?
fixing MTE-3095 bug by making the delete notifications distinguishable