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

Lut 26831 : simplify get response from backup #300

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

Timothee-Hx
Copy link
Contributor

@Timothee-Hx
Copy link
Contributor Author

Timothee-Hx commented Nov 21, 2023

This is a branch made from a fork of Lutece:26660 (since we are modifying the same file)

@Timothee-Hx Timothee-Hx changed the title Lut 26831 Lut 26831 : simplify get response from backup Nov 21, 2023
@Timothee-Hx
Copy link
Contributor Author

Timothee-Hx commented Nov 21, 2023

With lutece-database, in the login page url the "auth_provider", (url like this /lutece/jsp/site/Portal.jsp?page=mylutece&action=login&auth_provider=mylutece-database).
Before loading the backup responses, we check for "backup responses" if the request referer contains "auth_provider" or if user was already connected and haven't loaded the page before (_formResponseManager is null)

@@ -143,7 +139,7 @@ public class FormXPage extends MVCApplication
private static final String MARK_FORM_LIST = "form_list";
private static final String MARK_DISPLAY_CAPTCHA = "display_captcha";
private static final String MARK_CAPTCHA = "captcha";

private static final String AUTH_PROVIDER = "auth_provider";
Copy link
Member

Choose a reason for hiding this comment

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

the auth_provider parameter is external to the forms plugin, you should not use it in plugin forms code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The goal of this PR is to simplify the way we detecte if we just logged in as an FO user, because the way it was done was quite complexe.
By using the the referer of the request, we know the url, and we can check the presence of key work.

So I can make some proposal :

  • We could put this constant (auth_provider) in src/java/fr/paris/lutece/plugins/forms/service/entrytype/EntryTypeMyLuteceUser.java , since it's already tied up to mylutece.
  • We can check if the url contains "action=login" or just "login"
  • We can deduce that it could come from a login if the url doesn't contains "forms" and "admin".

Let me know what you propose

Copy link
Member

Choose a reason for hiding this comment

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

you cannot rely on the auth_provider attribute to know if a user has just connected because this attribute cannot always be retrieved from the request (a user can authenticate well before accessing the form)
can you explain why you want to do know if the user came to the login page ?
Wouldn't it be enough to test if the _formResponseManager object is empty?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"Wouldn't it be enough to test if the _formResponseManager object is empty?"
In the form's configuration if you check "Enable response backup" without "Authentication required", you don't go to the login page first, but directly to the form. When you have loaded page of the form _formResponseManager is not null, but you supposed to be able to loggin and get you backup responses whenever.

Detecting that you are coming from a login page thought the referer of the request, is much more simple than deducing it.

I have updaded this PR : implementing the login page parameters in forms.properties ( forms.frontOffice.loginPage.parameters=page=mylutece&action=login&auth_provider=mylutece-database ).
So the plugin user can change it, if the auth providers is not mylutece.

@Timothee-Hx Timothee-Hx force-pushed the LUT-26831 branch 2 times, most recently from e851626 to da491f0 Compare December 5, 2023 09:58
import fr.paris.lutece.plugins.genericattributes.service.entrytype.AbstractEntryTypeImage;
import fr.paris.lutece.plugins.genericattributes.service.entrytype.EntryTypeServiceManager;
import fr.paris.lutece.plugins.genericattributes.service.entrytype.IEntryTypeService;
import fr.paris.lutece.plugins.genericattributes.business.*;
Copy link
Member

Choose a reason for hiding this comment

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

Why include the importation of all classes from the business and entrytype package?

* @return the created {@code FormResponseManager} object
*/
public FormResponseManager createFormResponseManagerFromBackUp( Form form, String strUserGuid )
public FormResponseManager createFormResponseManagerFromBackUp( Form form, String strUserGuid, FormResponseManager formResponseManager )
Copy link
Member

Choose a reason for hiding this comment

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

Have you conducted an assessment of potential regressions resulting from the modification of method signatures?

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

Successfully merging this pull request may close these issues.

5 participants