jhipster / generator-jhipster

JHipster is a development platform to quickly generate, develop, & deploy modern web applications & microservice architectures.
https://www.jhipster.tech
Apache License 2.0
21.54k stars 4.02k forks source link

Make notification interceptor go silent #9073

Closed tibistibi closed 5 years ago

tibistibi commented 5 years ago
Overview of the feature request

The notification interceptor will add a notification like 'this thing is saved' But sometime i need it to be a silent save, i want the frontend to be in control.

Motivation for or Use Case

I have a form and do intermediate saves, these saves should be silent but the last save should have a massage.

Related issues or PR

As done here: https://stackoverflow.com/questions/46469349/how-to-make-an-angular-module-to-ignore-http-interceptor-added-in-a-core-module The notification interceptor can be changed like this so there is controle from the service:

  notification.interceptor.ts
        import { JhiAlertService } from 'ng-jhipster';
        import { HttpInterceptor, HttpRequest, HttpResponse, HttpHeaders, HttpHandler, HttpEvent } from '@angular/common/http';
        import { Injectable } from '@angular/core';
        import { Observable } from 'rxjs';
        import { tap } from 'rxjs/operators';
        // add this header to skip this interceptor
        export const InterceptorSkipHeader = 'X-Skip-NotificationInterceptor';
        export const SKIP_NOTIFICATION_HEADER = new HttpHeaders().set(InterceptorSkipHeader, '');

        @Injectable()
        export class NotificationInterceptor implements HttpInterceptor {
            constructor(private alertService: JhiAlertService) {}

            intercept(request: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> {
                if (request.headers.has(InterceptorSkipHeader)) {
                    const headers = request.headers.delete(InterceptorSkipHeader);
                    return next.handle(request.clone({ headers }));
                }
                return next.handle(request).pipe(
                    tap(
                        (event: HttpEvent<any>) => {
                            if (event instanceof HttpResponse) {
                                const arr = event.headers.keys();
                                let alert = null;
                                let alertParams = null;
                                arr.forEach(entry => {
                                    if (entry.toLowerCase().endsWith('app-alert')) {
                                        alert = event.headers.get(entry);
                                    } else if (entry.toLowerCase().endsWith('app-params')) {
                                        alertParams = event.headers.get(entry);
                                    }
                                });
                                if (alert) {
                                    if (typeof alert === 'string') {
                                        this.alertService.success(alert, { param: alertParams }, null);
                                    }
                                }
                            }
                        },
                        (err: any) => {}
                    )
                );
            }
        }

Now in any service import this:

import { SKIP_NOTIFICATION_HEADER } from 'app/blocks/interceptor/notification.interceptor';

and add the header to a call like this:

.put<IRating>(this.updateUrl, copy, { observe: 'response', headers: SKIP_NOTIFICATION_HEADER })

If wanted i could add a PR, i would need with help creating a test for it. I have it working in my application.

wmarques commented 5 years ago

@tibistibi LGTM, are you willing to do a PR on this ?

pascalgrimaud commented 5 years ago

I don't understand the feature. If you don't want notification, simply removing <jhi-alert></jhi-alert> at https://github.com/jhipster/jhipster-sample-app/blob/master/src/main/webapp/app/entities/operation/operation.component.html#L11 is not enough ?

tibistibi commented 5 years ago

@pascalgrimaud the feature is i want the have the alert on the last save someone makes but not on any intermediate saves. so lets say i have form which saves after every field is changed i do not want 10 alert messages, i only want an alert message after the user clicks the save button.

tibistibi commented 5 years ago

@wmarques yes i can make a PR on this only need help if this needs a test

pascalgrimaud commented 5 years ago

I suggest to wait other points of view, from the team. I'm not sure if it should be supported inside JHipster like this: managing the notification using header of the API is really strange.

tibistibi commented 5 years ago

agree. if there is a better way i would like to hear it off course.

amatosg commented 5 years ago

let's say I have an invoice where I save a number of invoice lines and every invoice line has products for which I want to update their stock value. I want to see only the invoice save/update notification, not the other notifications (those are being called intermediate saves), those would have a silent save/update

vishal423 commented 5 years ago

I believe, we should wait for https://github.com/angular/angular/issues/18155 to be closed (best is to upvote on that issue to help prioritize). That would help to have generic solution without involving temporary hacks and would make generator code more stable. Suggested workarounds to use temporary Header or to directly inject HttpBackend doesn't seems clean and can be specialized solution in application.

tibistibi commented 5 years ago

that one has been alive from Jul 16, 2017 so i could be a long wait, but i do not mind waiting i have the solution made already, the only reason i post it here is to help others

vishal423 commented 5 years ago

If we still want to go ahead, then, I think it would be clean to move all interceptors to ng-jhipster and have uniform way to apply or skip across all (few uses URI check at moment).

pascalgrimaud commented 5 years ago

I'm closing this for the following reason:

markvr4 commented 5 years ago

I think this issue is worth considering. The notifications/alerts have a significant impact on the UI, and more customization options (or at least documentation) would be welcome. The responses above mention that controlling alerts with headers is messy, but aren't alerts already controlled by headers using x-[app name]app-params and x-[app name]app-alert headers? I'm in the process of trying to find where those headers are set so I can do some customization, which is what led me to this post.

pascalgrimaud commented 5 years ago

You can remove the default jhi-alert and write your custom alert. I don't see what we can do more in the generated project

naris commented 1 year ago

I don't see why @pascalgrimaud is insisting that developers should remove ALL alert functionality from jhipster generated applications, and have those application constantly alert the user anytime anything is updated and created instead of correcting the bad design of jhipster and allowing alerts no not be mandatory for every action. If the developers have to disable and re-write code why use jhipster to generate anything?

pascalgrimaud commented 1 year ago

@naris : because the generated code is not adapted to all users. Some users want this, some other don't.

For me, it's easier and faster to generate and change the code, than to code directly the feature inside generator-jhipster.

Anyway, the main problem is to code the feature and to maintain it. You would have to do it for angular, react, vue. Do you want to discuss about this and to contribute @naris ? Then, to maintain the option in several months when it needs?

pascalgrimaud commented 1 year ago

@naris : another option would be to code your blueprint with this option. So as soon as it is coded, don't hesitate to link your repository here.

naris commented 1 year ago

@pascalgrimaud the problem is that the generated java code always adds x-app-alert headers that then get intercepted to display a message and there is no way to alter that other than to modify the generated code. Then, every single time you update something and have to generate the code again, the alterations get removed and you have to replace them.

A good design would have had a mechanism to enable or disable these headers/notifications with each call to create or update them, across all of the standard blueprints, without having to change generated code or write a blueprint.

I managed to work around this by adding a noalert request parameter to the Resource.java code and Service.ts angular/primeng code that could be set, when calling create(), update() or partialUpdate() from the front end. However, a mechanism such as this should have already existed in the jhipster generated code. Since it doesn't, generating new code with JHipster will be a royal pain :(

pascalgrimaud commented 1 year ago

@naris : I understand your point of view. I'd suggest to open a new ticket with a link to this one (as this one is too old), and discuss about this feature there.

If it looks ok and accepted, you can start to contribute directly into generator-jhipster

Sounds good?

naris commented 1 year ago

@pascalgrimaud I will see if I can make the time to do that. I do have an actual job, and a family, that takes most of my time so I will not likely have time for a while.

Also, I am not sure if I will be able to work directly on generator-jhipster as it is a really fragile project and difficult to get working without making my computer unable to be used to work on the projects for my employer.

pascalgrimaud commented 1 year ago

@naris : no problem, it's open source and all contributors here work on their free time. I already said it: it's far easier to generate the project and edit it, according to your needs :-) because no one will contribute and maintain this feature if it's not a must have