Skip to content

[4.0] Saving article cause : hits = '' is not a valid integer#30523

Merged
laoneo merged 15 commits intojoomla:4.0-devfrom
shim-sao:4.0-dev
Sep 9, 2020
Merged

[4.0] Saving article cause : hits = '' is not a valid integer#30523
laoneo merged 15 commits intojoomla:4.0-devfrom
shim-sao:4.0-dev

Conversation

@shim-sao
Copy link
Copy Markdown
Contributor

@shim-sao shim-sao commented Aug 31, 2020

Pull Request for Issue # .

Summary of Changes

Add default value 0 to the "hits" field in the admin form of the article.

Testing Instructions

Create an article, duplicate it.
In this case trying to save the duplicated article cause : '' is not a valid integer on save
The field hits is blank and is not be set to a valid integer in the query

This not happen all the time. Why ?

An integer must have a default valid value if it is save in the database or be set to NULL if it is allowed in the table

Actual result BEFORE applying this Pull Request

Saving article cause : '' is not a valid integer

Expected result AFTER applying this Pull Request

Article saved with default hits to 0

Documentation Changes Required

@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels Aug 31, 2020
@shim-sao shim-sao changed the title [4.0] Saving article cause : '' is not a valid integer [4.0] Saving article cause : hits = '' is not a valid integer Aug 31, 2020
@BertaOctech
Copy link
Copy Markdown

Hi.

I have not been able to reproduce the issue.

I have created 2 articles, I have visited only one of them. In this way I have one article with hits = 0 and another one with hits > 0.

In both cases, when I used "Save as Copy" a new article was created, and the message "Article saved." appeared.

I have tried in Chrome and Firefox.


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

@joomla-cms-bot joomla-cms-bot removed the NPM Resource Changed This Pull Request can't be tested by Patchtester label Sep 5, 2020
@Quy
Copy link
Copy Markdown
Contributor

Quy commented Sep 6, 2020

I have tested this item ✅ successfully on a5588af

To reproduce:

Edit an article.
Go to Publishing tab.
Use browser's inspector to change the value of Hits to empty string.
Click Save button.


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

@ceford
Copy link
Copy Markdown
Contributor

ceford commented Sep 7, 2020

I have tested this item ✅ successfully on a5588af

Followed the procedure given by BertaOctech


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

@Quy
Copy link
Copy Markdown
Contributor

Quy commented Sep 7, 2020

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Sep 7, 2020
@shim-sao
Copy link
Copy Markdown
Contributor Author

shim-sao commented Sep 7, 2020

Indeed this is the fastest way to reproduce the error.
I have only described in which case it occurred on my side ... it is not systematic as I specified.

I made the most obvious correction but for me there is clearly a problem in the form validation process on the server side.
Since the field is defined as an integer, even without a default value, it should be initialized to 0 no matter what, '' is not an integer.

Maybe there is an hidden error in the integer definition filter.

I think if this happens for this field, then all forms with none initialized integer should be reviewed.


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

@shim-sao
Copy link
Copy Markdown
Contributor Author

shim-sao commented Sep 8, 2020

This error is no need to be reproduce for this patch but...

I checked the category hits field and the default to 0 is present.

If you remove the default of the field 'hits' (category or article) and follow the @Quy instruction, you will get the error.
You can also enter a string instead of an integer and the string will be convert to empty string.
Incorrect integer value: '' for column 'hits' at row 1

This kind of error must not happen on the server side validation because everybody can make a form like he want, specially with the API

The form validation does not make its job if a value is set to string for an integer with no default, the default should be 0 on all case or the form is not valid


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

Add the last blank line
@laoneo laoneo merged commit e7f3d18 into joomla:4.0-dev Sep 9, 2020
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Sep 9, 2020
@laoneo
Copy link
Copy Markdown
Member

laoneo commented Sep 9, 2020

Thanks!

@laoneo laoneo added this to the Joomla 4.0 milestone Sep 9, 2020
sakiss pushed a commit to sakiss/joomla-cms that referenced this pull request Oct 16, 2020
…#30523)

* [4.0] fix Templates : Edit style - unsaved menu assignments

* Correct variable $userId not $userid in save function

* Update administrator/components/com_templates/src/Model/StyleModel.php

Co-authored-by: SharkyKZ <sharkykz@gmail.com>

* Update administrator/components/com_templates/src/Model/StyleModel.php

Co-authored-by: SharkyKZ <sharkykz@gmail.com>

* Update administrator/components/com_templates/src/Model/StyleModel.php

Co-authored-by: Richard Fath <richard67@users.noreply.github.com>

* Update administrator/components/com_templates/src/Model/StyleModel.php

Co-authored-by: Richard Fath <richard67@users.noreply.github.com>

* [4.0] Resizing template editor causes multiple editor instances joomla#22778

Tested also by clicking on add readmore and other actions

* Saving article cause : '' is not a valid integer

Default hits value should be a valid inetger to be saved in the database.

* Add last blank line

Add the last blank line

Co-authored-by: SharkyKZ <sharkykz@gmail.com>
Co-authored-by: Richard Fath <richard67@users.noreply.github.com>
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.

6 participants