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.

Sign up for GitHub

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

Jump to bottom

fix: Update web notification from gtmpl to vue - EXO-66413 #331

Merged
merged 5 commits into from
Dec 7, 2023
Merged

fix: Update web notification from gtmpl to vue - EXO-66413 #331

merged 5 commits into from
Dec 7, 2023

Conversation

hakermi
Copy link
Member

@hakermi hakermi commented Nov 28, 2023

This PR updates the web notification old gtmpl templates to vue & vuetify for better UX

aycherif and others added 3 commits November 24, 2023 00:18
@aycherif @exo-swf
TASK-62301: Create FB maintenance and update projects versions/depend…
8d46154 
…encies
@akhanfir @exo-swf
fix: [Process]Issue with the values of the process quick filter - EXO…
751041a 
…-62308 (#273)

Prior to this change, the process quick filter is not working properly. After this changes, the process quick filter displays the workflows correctly.
@akhanfir @exo-swf
fix: [Process]Issue with the values of the process quick filter EXO-…
2cb588b 
…62308 (#285)

This PR performs #273. In this pr deleted the change applied on the label order in quick filter in the previous pr .
@hakermi hakermi requested a review from ahamdi November 28, 2023 08:36
rdenarie
rdenarie reviewed Nov 28, 2023
View reviewed changes
...ices/src/main/java/org/exoplatform/processes/listener/CancelRequestNotificationListener.java
Comment on lines +21 to +22
ctx.append(NotificationArguments.REQUEST_PROCESS, projectDto.getName());
ctx.append(NotificationArguments.REQUEST_TITLE, taskDto.getTitle());
Copy link
Member

Choose a reason for hiding this comment

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

I see you add parameters in the context of the notification.

So, notifications created BEFORE your modification will not have the same parameters than notification created AFTER your modifications

Can you ensure that notifications created BEFORE will not break the interface (js error for example) at least. In the better case, notifications created before can display a small set of informations even if all parameters are not present

Copy link
Member Author

Choose a reason for hiding this comment

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

No js error or display break, i added new params only for CancelRequestPlugin but here is the display

old notif display =>
cancelnotifbefore

new notif display
cancel_notif_after

Copy link
Member Author

@hakermi hakermi Nov 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

we can improve it by adding condition on existing params and display the old message. wdyt?

Copy link
Member

Choose a reason for hiding this comment

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

yes please, if it is possible I prefer to make both possibility works

Copy link
Member Author

Choose a reason for hiding this comment

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

done https://github.com/exoplatform/processes/compare/30fc51145f9633622e02e9791ae48e7f85c0b3ac..4d7f083a359ac61e82f4c7bcf557a3e5cfe91f85

Copy link
Member

Choose a reason for hiding this comment

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

@rdenarie is ok on your part following @hakermi latest feedback ?

Copy link
Member

Choose a reason for hiding this comment

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

yes it is

@hakermi
fix: Update web notification from gtmpl to vue - EXO-66413
4d7f083 
This PR updates the web notification old gtmpl templates to vue & vuetify for better UX
@ahamdi ahamdi removed their request for review November 28, 2023 09:46
rdenarie
rdenarie previously approved these changes Dec 7, 2023
View reviewed changes
@hakermi hakermi enabled auto-merge (squash) December 7, 2023 13:03
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Dec 7, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

50.0% 50.0% Coverage
0.0% 0.0% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@hakermi hakermi merged commit 27d1730 into feature/maintenance Dec 7, 2023
6 of 7 checks passed
@hakermi hakermi deleted the TASK-66413 branch December 7, 2023 13:12
Jihed525 pushed a commit that referenced this pull request Dec 7, 2023
@hakermi
fix: Update web notification from gtmpl to vue - EXO-66413 (#331)
650b2e6 
This PR updates the web notification old gtmpl templates to vue & vuetify for better UX

(cherry picked from commit 27d1730)
Jihed525 pushed a commit that referenced this pull request Dec 7, 2023
@hakermi
fix: Update web notification from gtmpl to vue - EXO-66413 (#331)
7e43adb 
This PR updates the web notification old gtmpl templates to vue & vuetify for better UX

(cherry picked from commit 27d1730)
Jihed525 pushed a commit that referenced this pull request Dec 7, 2023
@hakermi @Jihed525
fix: Update web notification from gtmpl to vue - EXO-66413 (#331)
72fee4c 
This PR updates the web notification old gtmpl templates to vue & vuetify for better UX

(cherry picked from commit 27d1730)
exo-swf pushed a commit that referenced this pull request Dec 8, 2023
@hakermi @exo-swf
fix: Update web notification from gtmpl to vue - EXO-66413 (#331)
9bee7ed 
This PR updates the web notification old gtmpl templates to vue & vuetify for better UX
Jihed525 pushed a commit that referenced this pull request Dec 8, 2023
@hakermi @Jihed525
fix: Update web notification from gtmpl to vue - EXO-66413 (#331)
3a848ac 
This PR updates the web notification old gtmpl templates to vue & vuetify for better UX

(cherry picked from commit 27d1730)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Jihed525 Jihed525 Jihed525 approved these changes

@rdenarie rdenarie Awaiting requested review from rdenarie

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@hakermi @rdenarie @Jihed525 @aycherif @akhanfir