Skip to content

Conversation

@durubayram
Copy link

Pull Request for Issue #21249 .

Summary of Changes

by adding a new error type(COM_CONFIG_ERROR_VALIDATION) in ApplicationController.php file, you're going to be able to see description why form validation failed instead of getting an error which is 'No database selected'

Testing Instructions

Edit the Global configuration and provide an incorrect value for "session lifetime" : 19999
(it is possible to bypass the maximum value of the input field, which is 16383)

Expected result

The configuration is not saved and the user get a error message with the explanation of the issue which is 'Form validation failed. Please check the form.'
ekran goruntusu 83

@ghost
Copy link

ghost commented Aug 3, 2018

I have tested this item 🔴 unsuccessfully on 35e8d5b

19999 is saved and Message shows Constant.

Please ignore Test as for now Front- and Backend-PRs are able to test by Patchtester.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/21384.

@SharkyKZ
Copy link
Contributor

SharkyKZ commented Aug 3, 2018

Use error from model:
$this->app->enqueueMessage($model->getError(), 'error');

@ghost
Copy link

ghost commented Aug 3, 2018

I have not tested this item.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/21384.

@ghost
Copy link

ghost commented Aug 3, 2018

I have tested this item 🔴 unsuccessfully on 35e8d5b

19999 is saved and Message shows Constant.

screen shot 2018-08-03 at 15 02 46


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/21384.

@Twincarb
Copy link
Contributor

Twincarb commented Aug 3, 2018

Is the PR missing the string being added to the language file?

@wojsmol
Copy link
Contributor

wojsmol commented Aug 3, 2018

@Twincarb yes


// Redirect back to the edit screen.
$this->setRedirect(Route::_('index.php?option=com_config', false));
$this->app->enqueueMessage(\JText::_('COM_CONFIG_ERROR_VALIDATION'), 'error');
Copy link
Contributor

Choose a reason for hiding this comment

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

please use the namespaced code so this should be Text and not \JText

// Redirect back to the edit screen.
$this->setRedirect(Route::_('index.php?option=com_config', false));
$this->app->enqueueMessage(\JText::_('COM_CONFIG_ERROR_VALIDATION'), 'error');
$this->app->redirect(\JRoute::_('index.php?option=com_config', false));
Copy link
Contributor

Choose a reason for hiding this comment

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

please use the namespaced code so this should be Route and not \JRoute

@joomla-cms-bot joomla-cms-bot added the Language Change This is for Translators label Aug 4, 2018
@wojsmol
Copy link
Contributor

wojsmol commented Aug 4, 2018

FILE: ...dministrator/components/com_config/Controller/ApplicationController.php
--------------------------------------------------------------------------------
FOUND 2 ERROR(S) AFFECTING 2 LINE(S)
--------------------------------------------------------------------------------
 122 | ERROR | Tabs must be used to indent lines; spaces are not allowed
 123 | ERROR | Tabs must be used to indent lines; spaces are not allowed
--------------------------------------------------------------------------------
UPGRADE TO PHP_CODESNIFFER 2.0 TO FIX ERRORS AUTOMATICALLY
--------------------------------------------------------------------------------

// Redirect back to the edit screen.
$this->setRedirect(Route::_('index.php?option=com_config', false));
$this->app->enqueueMessage(Text::_('COM_CONFIG_ERROR_VALIDATION'), 'error');
$this->app->redirect(\JRoute::_('index.php?option=com_config', false));
Copy link
Contributor

Choose a reason for hiding this comment

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

use Route here

@joomla-cms-bot joomla-cms-bot changed the title it shows that description of error to add new error type [4.0] it shows that description of error to add new error type Aug 4, 2018
@wilsonge
Copy link
Contributor

wilsonge commented Aug 6, 2018

Codestyle fixed

Copy link
Contributor

@brianteeman brianteeman left a comment

Choose a reason for hiding this comment

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

approve language changes

@brianteeman
Copy link
Contributor

I have tested this item ✅ successfully on 1a3c4eb


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/21384.

@ghost
Copy link

ghost commented Aug 6, 2018

I have tested this item 🔴 unsuccessfully on 1a3c4eb

Language String shown correct, but incorrect value for "session lifetime" "19999" is saved

System information

  • Nightly Build 4.0.0-alpha5-dev
  • Multilingual Sample Data (French, German DE, Persian)
  • Template: Cassiopeia
  • macOS Sierra, 10.13.6
  • Firefox 61 (64-bit)

CloudAccess.net

@obsidev
Copy link
Contributor

obsidev commented Aug 6, 2018

Hi,

The "session lifetime" is not saved in the configuration ; it is coming from the session which store the form data to not loose what the user was editing.
So the user know that the configuration is not saved but its form is kept which allow him to fix what is wrong (but he need to know what is wrong)

There is a comment saying that the model should display the error message (which is not the case and we don't know why).

The validate method enqueued all messages for us, so we just need to redirect back.

Letting the model display the errors or pushing the errors from the model is a requirement because the user must know what is going from with the provided data.
The patch fix the issue but it would require further improvements to have a good user experience.

Regards,

@ghost ghost added the J4 Issue label Apr 5, 2019
@ghost ghost removed the J4 Issue label Apr 13, 2019
@rmittl
Copy link

rmittl commented Oct 19, 2019

Using "Blanks" before or after the value is accepted.
No error message - the value after saving is 15.
Example " 18005", "18005 " is value "15" after saving

Proposed solution, trim before check.

@vijaykhollam
Copy link
Contributor

vijaykhollam commented Oct 19, 2019

I have tested this item 🔴 unsuccessfully on 88726e0

#
Language String is shown correct, but incorrect value for "session lifetime" "19999" is saved

System information
Joomla! 4.0.0-beta1-dev Development [ Amani ] 17-October-2019 20:21 GMT
Web Server : Apache
WebServer to PHP Interface : cgi-fcgi
PHP 7.3.8
MySQL 5.7.23-cll-lve


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/21384.

@pravinTek
Copy link

Hi,
Now language string showing a correct message and also incorrect value for "session lifetime" "19999" is not storing in config.php
But In the backend UI System -> Global Configuration still showing "19999" this value under "session lifetime".

@Quy
Copy link
Contributor

Quy commented Jan 2, 2020

This is no longer an issue.

21384

@Quy Quy closed this Jan 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.