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

Allow redirect via Annotation on action #1554

Open
tandraschko opened this issue Feb 17, 2021 · 12 comments
Open

Allow redirect via Annotation on action #1554

tandraschko opened this issue Feb 17, 2021 · 12 comments

Comments

@tandraschko
Copy link

tandraschko commented Feb 17, 2021

Currently the return value of the action triggers a navigation

public String action() {
    return "myNewView.xhtml"
}

to force a redirect, we have to add a string param:

public String action() {
    return "myNewView.xhtml?faces-redirect=true"
}

Wouldnt it be nice if we allow:

@Redirect
public String action() {
    return "myNewView.xhtml"
}

and

@Redirect("myNewView.xhtml")
public void action() {

}
@edwin-amoakwa
Copy link

Will this also be applicable to void methods e.g.

@Redirect public void action() { doSomeOtherAction(); }

@manorrock
Copy link

I rather see it only apply to void methods and then taking an argument

@tandraschko
Copy link
Author

tandraschko commented Feb 17, 2021

We could even allow both, see my updated example.

@BalusC
Copy link
Member

BalusC commented Feb 25, 2021

I think this is great if it also allows external URLs.

Old

public void action() throws IOException {
    // ...
    FacesContext.getCurrentInstance().getExternalContext().redirect("https://google.com");
}

New (just match ^https?://)

@Redirect("https://google.com")
public void action() { // Look ma, no throws!
    // ...
}

And EL expressions.

Old

public String action() {
    // ...
    boolean flag = compute();
    return flag ? "foo" : "bar";
}

New

private boolean flag;

@Redirect("#{bean.flag ? 'foo' : 'bar'}")
public void action() {
    // ...
    flag = compute();
}

public boolean isFlag() {
    return flag;
}

@arjantijms arjantijms added this to the 4.0 milestone Mar 27, 2021
@arjantijms arjantijms added the 4.0 label Mar 27, 2021
@tandraschko
Copy link
Author

will prototype this in MF as next task

@tandraschko
Copy link
Author

tandraschko commented Apr 14, 2021

What should we do with forward?
Either make a @Forward or @Navigate with redirect or forward param

@BalusC
Copy link
Member

BalusC commented Apr 14, 2021

I'd rather not implement/advocate a "forward" for now. It only encourages poor practices.

@tandraschko
Copy link
Author

Created a el-spec feature request, which would make this feature really easy to implement: jakartaee/expression-language#154

@tandraschko
Copy link
Author

tandraschko commented Apr 14, 2021

I currently think about something like that:

@Target(value = ElementType.FIELD)
@Retention(value = RetentionPolicy.RUNTIME)
public @interface Redirect
{
    /**
     * The relative or absolute path, which will be passed to the NavigationHandler.
     * EL-Expressions are also supported.
     * 
     * This parameter is required when the return-type of the method is void. 
     * 
     * @return 
     */
    String value() default "";

    boolean includeViewParams() default false;
}

which could be easily accessed in ActionListenerImpl

@tandraschko
Copy link
Author

EL feature is implemented now, i will do a prototype in MF next week

@tandraschko tandraschko removed this from the 4.0 milestone Nov 15, 2021
@tandraschko tandraschko changed the title Faces 4.0: Allow redirect via Annotation on action Allow redirect via Annotation on action Jun 9, 2022
@tandraschko tandraschko added this to the 4.1 milestone Jun 9, 2022
@tandraschko tandraschko removed the 4.0 label Jun 9, 2022
@tandraschko tandraschko modified the milestones: 4.1, 5.0 Feb 25, 2023
@tandraschko tandraschko removed this from the 5.0 milestone Jun 22, 2024
@tandraschko
Copy link
Author

@BalusC @arjantijms i have propably some time to work on it. Would you like to see it? im sure its only a small change, similar to my CDI bridges

@arjantijms
Copy link
Contributor

@tandraschko sounds good! Sure would like to see it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants