🐛 Fix: 'cancel' button redirects to console instead of portal #387

jbostoen · 2023-01-14T11:59:18Z

Before:

Sign in to the portal, navigate to https://demo.combodo.com/simple/pages/exec.php/object/create/UserRequest?exec_module=itop-portal-base&exec_page=index.php&portal_id=itop-portal , press Cancel.
User with console permissions is redirected to the back end instead of staying in the portal.

After PR: user with console permissions should stay in the portal. Either a simple 'go back 1 page' is executed, or the user is redirected to the portal's home page.

Molkobain · 2023-01-15T16:01:18Z

datamodels/2.x/itop-portal-base/portal/public/js/portal_form_handler.js

-				// Try to close the window
-				window.close();
+
+				if(window.history.length > 1) {


window.history.length greater than 1 means no history? Seems counter intuitive. Isn't it equals 0?

Absolutely right, should have been == 1 (it's always at least 1 = current location)

Molkobain · 2023-01-15T16:08:27Z

datamodels/2.x/itop-portal-base/portal/public/js/portal_form_handler.js

+					var regexPortal = new RegExp('(pages\\/exec\\.php)(.*?)(\\?exec_module=itop-portal-base&exec_page=index\\.php&portal_id=.*?)(&|$)');
+					var aRegexMatch = window.location.href.match(regexPortal);
+
+					if(aRegexMatch !== null) {
+
+						sHomepageUrl += aRegexMatch[1] + aRegexMatch[3];
+
+					}


I'm not sure hacking the URL through the regex is the best approach here. To me it's more like the this.options.base_url isn't set properly in the first place. It should be set with app['combodo.portal.base.absolute_url'] in datamodels/2.x/itop-portal-base/portal/templates/bricks/object/mode_create.html.twig

True, also some customers might have different path e.g. set up.

For example they use another portal base or portal_id is not the third argument or the portal url is rewritten..

True, but I think it will be a minority? (Perhaps you guys have a better view on that).

It addresses the most common implementation. About the order of arguments: I could rewrite it like that, but I'm not sure if there's a different order somewhere in the native implementation?

Anyway, is the RegEx needed? The filename (portal_form_handler.js) kind of suggests we are always in the portal?

I explained the fix as we would accept it in the first message :)

It definitely needs the portal ID parameter. The other parts I added to be more certain of where it was used and to re-use in the final URL

@Molkobain so I should just change this line to use app['combodo.portal.base.absolute_url'] instead in the create object Twig template?

base_url: "{{ app['combodo.absolute_url'] }}",

Alright, I updated the PR to be a mix of @Molkobain's suggestion and also just some basic 'go back' logic.

Thanks, that new behavior will need to be tested / validated. We'll see that during next functional review

Hipska · 2023-01-16T08:20:46Z

I'm wondering if this now also messes up when the form was created by going to the services browser. As if I do that, clicking cancel on the modal, the modal is closed. If I make the form open as a new page by right clicking on the link, It just closes the window by clicking on cancel (Safari & FF 102) So in this scenario, everything works as expected.

datamodels/2.x/itop-portal-base/portal/public/js/portal_form_handler.js

…andler.js Co-authored-by: Molkobain <lajarige.guillaume@free.fr>

…andler.js

Molkobain · 2023-02-13T13:05:38Z

Accepted during technical review.

Molkobain · 2023-02-14T16:27:57Z

Discussed during functional review:

Fix of the URL passed to the widget is accepted.
Change on the JS to go back to the previous page if any history is available is rejected. I missed it during the technical review, but this is done in the "apply close rule" function which is meant to close the page/form. Any redirection "back" to the previous page should be handle by the "back" redirection rule and is out of scope of this PR.

So long story short, revert the changes on the JS file and the PR will be good to merge :)

jbostoen · 2023-02-14T16:51:24Z

Thanks for the feedback!

Well, it doesn't make sense the page would be "closed" at some point.

Image this scenario:

User opens the portal. ( https://demo.combodo.com/simple/pages/exec.php/?exec_module=itop-portal-base&exec_page=index.php&portal_id=itop-portal )
User gets redirected by an extension to the actual URL: https://demo.combodo.com/simple/pages/exec.php/object/create/UserRequest?exec_module=itop-portal-base&exec_page=index.php&portal_id=itop-portal (non-modal).
Window should not be closed, IMHO?

Isn't the current logic only based on the assumption that the above URL is shown in a modal dialog (while this could be much more versatile, I'm using it outside a modal)?

Molkobain self-assigned this Jan 15, 2023

Molkobain added the bug Something isn't working label Jan 15, 2023

Molkobain added this to First review needed in Combodo PR dashboard via automation Jan 15, 2023

Molkobain reviewed Jan 15, 2023

View reviewed changes

Molkobain moved this from First review needed to Pending contributor update in Combodo PR dashboard Jan 15, 2023

jbostoen force-pushed the fix_portal_cancel branch from 5bd1860 to 8a36c42 Compare January 15, 2023 16:10

Hipska approved these changes Jan 16, 2023

View reviewed changes

jbostoen force-pushed the fix_portal_cancel branch from 8a36c42 to 3318570 Compare January 16, 2023 18:38

* Fix for 'cancel' button in portal

2b3f1ea

jbostoen force-pushed the fix_portal_cancel branch from 3318570 to 2b3f1ea Compare January 16, 2023 18:40

Molkobain requested changes Jan 17, 2023

View reviewed changes

Molkobain moved this from Pending contributor update to Pending technical review in Combodo PR dashboard Jan 17, 2023

jbostoen and others added 2 commits January 17, 2023 15:20

Update datamodels/2.x/itop-portal-base/portal/public/js/portal_form_h…

fc841aa

…andler.js Co-authored-by: Molkobain <lajarige.guillaume@free.fr>

Update datamodels/2.x/itop-portal-base/portal/public/js/portal_form_h…

79f5886

…andler.js Co-authored-by: Molkobain <lajarige.guillaume@free.fr>

jbostoen requested a review from Molkobain January 17, 2023 14:21

Update datamodels/2.x/itop-portal-base/portal/public/js/portal_form_h…

f15d3fb

…andler.js

Molkobain approved these changes Jan 17, 2023

View reviewed changes

Molkobain moved this from Pending technical review to Pending functional review in Combodo PR dashboard Feb 13, 2023

Molkobain moved this from Pending functional review to Pending contributor update in Combodo PR dashboard Feb 14, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🐛 Fix: 'cancel' button redirects to console instead of portal #387

🐛 Fix: 'cancel' button redirects to console instead of portal #387

jbostoen commented Jan 14, 2023

Molkobain Jan 15, 2023

jbostoen Jan 15, 2023

Molkobain Jan 15, 2023

Hipska Jan 16, 2023

jbostoen Jan 16, 2023

Hipska Jan 16, 2023

Molkobain Jan 16, 2023

jbostoen Jan 16, 2023

jbostoen Jan 16, 2023

Molkobain Jan 16, 2023

jbostoen Jan 16, 2023

Molkobain Jan 17, 2023

Hipska commented Jan 16, 2023 •

edited

Molkobain commented Feb 13, 2023

Molkobain commented Feb 14, 2023

jbostoen commented Feb 14, 2023

🐛 Fix: 'cancel' button redirects to console instead of portal #387

Are you sure you want to change the base?

🐛 Fix: 'cancel' button redirects to console instead of portal #387

Conversation

jbostoen commented Jan 14, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hipska commented Jan 16, 2023 • edited

Molkobain commented Feb 13, 2023

Molkobain commented Feb 14, 2023

jbostoen commented Feb 14, 2023

Hipska commented Jan 16, 2023 •

edited