I have tested this item ? unsuccessfully on b93b118

After PR applied got at /index.php?option=com_templates&view=templates&client_id=0

@franz-wohlkoenig

I have tested this item red_circle unsuccessfully on b93b118
After PR applied got at /index.php?option=com_templates&view=templates&client_id=0

Yes, this an expected behavior if you use PR tester for this because it contains SQL and javascript changes.

You, need to download this branch and make a fresh install of it then it should work fine.

You, need to download this branch and make a fresh install of it then it should work fine.

Thanks for Info. Can you please update Test Instructions so Vlunteers know it in your first Comment?

you please update Test Instructions so Volunteers know it in your first Comment?

Sure

Please look at the other examples in the core for how to do grid.checkall

@brianteeman Now fine?

yes that part is

Is there a reason that the diff view doesnt show line numbers?

See image below for suggested language string change
AND change from p class=lead to h2

@richard67 Thank you for your comment.
I think we need this file.
We thought about this question in our poject. You can see our issue here: joomla-projects/gsoc18_override_management#41

This file is not only for updating old entries, but also for checking if there is support in the database - In the task repair database.
https://github.com/joomla-projects/gsoc18_override_management/blob/01b7b917b851c25c271b56e19c6e79735c7e08b7/administrator/components/com_admin/script.php#L4094

@astridx I know what this file is good for because I am the one who invented it ;-)

Yes it was my mistake - it should indeed be overridden

Beside this I must say that in opposite to other PRs which have added new features to 4.0, this PR here looks very well done to me. The utf8mb4 conversion is correct, and the sql schema update scripts look ok to me, too.
As soon as all reviews have finished and changes have been implemented, let me know when it is ready for test. I have a holiday this week and so have time for testing.

@Anu1601CS please fix code style errors reported by Drone:
FILE: .../github.com/joomla/joomla-cms/components/com_tags/View/Tag/HtmlView.php
FOUND 2 ERROR(S) AFFECTING 2 LINE(S)
364 | ERROR | There must be exactly one blank line between descriptions in function comment
365 | ERROR | There must be exactly one blank line before the tags in function comment

@Anu1601CS please fix code style errors reported by Drone:
FILE: .../github.com/joomla/joomla-cms/components/com_tags/View/Tag/HtmlView.php
FOUND 2 ERROR(S) AFFECTING 2 LINE(S)
364 | ERROR | There must be exactly one blank line between descriptions in function comment
365 | ERROR | There must be exactly one blank line before the tags in function comment

@richard67 Thanks for the review but. This file is not a part of my PR so better to fix this in separate PR
@zero-24 can help here drone?

@Anu1601CS Ah, sorry, I did not notice that the file with the code style errors does not come from your PR.
Regarding the language changes I requested, it looks ok now.

Is there a reason that the diff view doesnt show line numbers?

@brianteeman Not big reason because jsdiff library does not support this feature. And, how it was going to help us. Like an editor number line of code change on the size of the browser which is also not stable. Like git

@zero-24 can help here drone?

Drone seams to be happy now :)

Thanks @zero-24

Oops sorry

@Anu1601CS Could you provide in the testing instructions download links to the 2 versions of the hello world component which you have used in the video? It would be helpful for testers.

Drone failure seems not to be related to this PR but to system test for libraries/vendor/joomla/test-system/src/acceptance/administrator/components/com_media/MediaListCest.php:createFolderUsingToolbarButton.

@zero-24 Maybe you can help again?

@Anu1601CS Could you provide in the testing instructions download links to the 2 versions of the hello world component which you have used in the video? It would be helpful for testers.

@richard67 I just updated description section > Testing

@franz-wohlkoenig It seems your test result is hanging somehow. Even after new commits have been made, the issue tracker still shows your unsuccessful test. Any idea what could be wrong with the issue tracker, or who could help? Or maybe you just set your test result to "not tested" in the issue tracker?

I have tested this item ✅ successfully on b93b118

Works as shown in the video.
Tested with MySQL DB.
Could not test with PostgreSQL DB because 4.0 install does not work on PostgreSQL DB for reasons not related to this PR.

I have tested this item ✅ successfully on b93b118

Works as shown in the video.
Tested with MySQL DB.
Could not test with PostgreSQL DB because 4.0 install does not work on PostgreSQL DB for reasons not related to this PR.

And no idea why my test result is shown twice in the issue tracker.

And no idea why my test result is shown twice in the issue tracker.

And no idea why my test result is shown twice in the issue tracker.

@richard67 Sometimes it happens. Thanks for testing

It seems your test result is hanging somehow. Even after new commits have been made, the issue tracker still shows your unsuccessful test.

Just fixed that.

Drone failure seems not to be related to this PR but to system test for libraries/vendor/joomla/test-system/src/acceptance/administrator/components/com_media/MediaListCest.php:createFolderUsingToolbarButton.

Yes does not seam to be related to this PR i have just rebooted the drone tests: http://ci.joomla.org/joomla/joomla-cms/9421/16 Lets see how far this goes than.

I will try to find time to review the strings tomorrow but from your video I can see some issues

In this case I think you mean "original" not "core"

Here the message is wrong - no overrides have been updated. Not sure yet what it should be but what you are saying is x files that you have overridden have changed and you may need to update the override. The current message doesnt convey that meaning

Here the message is wrong again. No "conflicts" have been found. What you are saying is that x files that you have overridden have changed and you may need to update the override. The current message doesnt convey that meaning

Finally (for now) your use of the published/unpublished icons in a column marked status to indicate successful is very confusing.

@zero-24 It seems that the issue tracker is hanging for this PR. My last test result has shown am old commit number (same as for franz several commits before), and now after the last commit my valid test result from befor that last commit is still shown. Maybe this PR just has too many commits so the issue tracker can't follow it anymore?
Anyway, my last test result is still valid because commit after that was just a doc block change.

@zero-24 It seems that the issue tracker is hanging for this PR. My last test result has shown am old commit number (same as for franz several commits before), and now after the last commit my valid test result from befor that last commit is still shown. Maybe this PR just has too many commits so the issue tracker can't follow it anymore?
Anyway, my last test result is still valid because commit after that was just a doc block change.

@astridx @laoneo Need some opinion. I also think its a little bit confusing.

We have to notify that the core file of override is updated. The user can mark this override history checked or unchecked. And notification in control panel should show number of override core files updated.
@brianteeman Can you please suggest your language strings to clearly express these things ?

@Anu1601CS

I will try to find time to review the strings tomorrow

Thanks @brianteeman :)

It is always good when people do review with fresh eyes as you and we mentors are always to deep in the mud already. The points from @brianteeman are valid and I would implement them.

Updated the list of core extensions joomla-projects/gsoc18_override_management#71

Code style joomla-projects/gsoc18_override_management#72

Code style joomla-projects/gsoc18_override_management#73

@brianteeman The 2 codestyle issues (ordering of use statements) have already been fixed here in this PR. Will there not be merge conflicts if it is also fixed in the project repository and then someone wants to merge back changes from there into here?

Change icon for consistency joomla-projects/gsoc18_override_management#74

it is not the override that has been updated

Maybe "Overridden file(s) updated?"

No - that still says that its the override that has been updated not the original file that you have created an update for

Note the \ after html when it should obviously be /

Or maybe "Original(s) of Override(s) updated"

Yes, but then with lowercase in the middle of the sentence, i.e. "Original(s) of override(s) updated"

But that could be too long for the button.

@Anu1601CS Some language changes as promised joomla-projects/gsoc18_override_management#76

link.setAttribute('href', 'index.php?option=com_plugins&task=plugin.edit&extension_id=491');

Really not a fan of hard coding the extension_id here. Can't we use a helper like we do elsewhere to get the id

I have not tested this item.

Resetting my test result as long as review is ongoing and changes are expected.

@Anu1601CS Some language changes as promised joomla-projects/gsoc18_override_management#76

@brianteeman Thank you very much :)

For the quick icon text I think that this will work
"X overrides to check"

For the quick icon text I think that this will work
"X overrides to check"

Yes this is also good.

@Anu1601CS I think it is the clearest and least ambiguous we can come up with for now. I will leave it to you to make the update

X overrides to check joomla-projects/gsoc18_override_management#77

X overrides to check joomla-projects/gsoc18_override_management#77

Thanks but already done here.

please try and keep your branch and this pr in sync. makes it hard to help otherwise

please try and keep your branch and this pr in sync. makes it hard to help otherwise

Ok

please try and keep your branch and this pr in sync. makes it hard to help otherwise

@brianteeman Now Done! :)

Please test installing a language pack eg French

0 Call to undefined method Joomla\Component\Templates\Administrator\Model\TemplateModel::getCoreList()

The problem is in this function

	public function getOverrideCoreList()
	{
		// Get template model
		$templateModel = $this->getModel();
		$result = $templateModel->getCoreList();

		return $result;
	}

Please test installing a language pack eg French

0 Call to undefined method Joomla\Component\Templates\Administrator\Model\TemplateModel::getCoreList()

The problem is in this function

public function getOverrideCoreList()
{
	// Get template model
	$templateModel = $this->getModel();
	$result = $templateModel->getCoreList();

	return $result;
}

@brianteeman I am not able to reproduce issue.

I tested in this manner.

1. System -> install languages

Sorry something was missing from my instructions as I tried to simplify it.

I was able to create the problem when updating a language and when installing a language using the install from url. I can now replicate it all the time.

Tomorrow I will try again with a clean install - I am speaking at a user group tonight

@brianteeman Any update ?

I tested this PR successful.

1. I changed to this branch and made a new installation.

• git fetch joomla pull/21851/head:gsoc
• git checkout gsoc
• composer install
• npm install
• rm configuration.php
• open a browser and start installation

2. After installation I see

• the quickicon in the control panel,
• the two plugins (/administrator/index.php?option=com_plugins&view=plugins) and
• the view that will list the overridden files whose originals were update in one of the last updates (http://localhost/gsoc18/administrator/index.php?option=com_templates&view=template)

3. I installed a template, a module and a component. (This extensions are only for testing this PR.)
   com_agosms-1.0.27.zip
   mod_agosm-1.0.27.zip
   tpl_agfirework_20180624_1525.zip

4. I created an override (and edit this override) of the view agosm in the component for one template (cassiopaia) and an override of the module view in the other template (agfirework).

NOTE:
Sometimes the tabs in template manager are not displayed correctly. You see the content of all tabs on one page. This is not an issue of this pr. If I click on every tab the display is correct again.

5. I see the changes in the "diff view". I see nothing in the "updated files" view, because here the files are only added via update after on one original is updated.
   

6. I made an update of the module via Joomla! Updater and a new install of v.28 of the component with this file:
   com_agosms-1.0.28.zip
   
   
   

7. I checked in template manager the new view updated files (/administrator/index.php?option=com_templates&view=template). Everything is fine. The file that is overridden in template cassiopeia is shown only in this template and the file that is overridden in template agfirework is shown in the view of this template.
   
   

8. In control panel I see a red quickicon. If I deactivate the Installer - Override plugin I see a message in the quickicon and if I deactivate the Quick Icon - Joomla! Overrides Update Notification plugin I see no quickion. That is all fine.
   
   

9. I am able to check and unckeck an entry in the view updated list. After an action I am redirected to the fist tab. But this is also not an issue of this pr. I think it is a problem of the backend template.
   

10. If I check all entries in the view updated list in all templates the quickicon is green again.

11. I am able to delete a file in the view updated list via toolbar button "delete".
    
    

Awesome detailed report - thanks - I will repeat the tests myself tomorrow

The tab issues are indeed unrelated

The tab issues are indeed unrelated

Yes, should i do PR for tab issue @brianteeman

The tab issue has an open tracker item for it #21855

@astridx please mark your Test as successfully:

• open Issue Tracker
• Login with your github-Account
• Click on blue "Test this"-Button above Authors-Picture
• mark your Test as successfully
• hit "submit test result"

Now I was able to test a Joomla Update. Thanks to @zero-24 who provided a joomla update especially for this test. In this Joomla Update only the view article in com_content is changed.

1. I made a new installation on this branch like explained in my last comment.

2. Then I created an override for com_content view article.
   

3. After that I set a custom url for joomla update. The custom url is https://www.jah-tz.de/downloads/core/gsoc18_override_management/gsoc18_update_list.xml
   

4. When I did this I made a joomlaupdate via https://www.jah-tz.de/downloads/core/gsoc18_override_management/gsoc18_update_list.xml
   
   

5. Back in template manger I see the changes in diff view and updated files view
   
   

6. In control pane I see the red quickicon. That is fine. Only the messages could be improved :).
   

I have tested this item ✅ successfully on b93b118

@franz-wohlkoenig
I have now correctly marked the test as successful. Since I was involved in the project, others have to decide if it counts.

@richard67
It would be great if you could repeat your test.
If you and @brianteeman also tests successful we would have three :)

Extension id 491 is already used in 3.9 for plg_privacy_content (for a clean install)
That's allright as this PR can be modified to take this into account later for the sql but I agree with
#21851 (comment) that we should not be using a hardcoded id in the js.

@infograf768 Thank you for your opinion. You're right.
But: This PR is complex and it is time consuming to repeat all tests many times. What do you think. Would it be OK to open an issue for "not using a hardcoded id for the plugin" in the case this pr get merged? And we can work later on it?
Advantage: The functionality of this new feature is well tested and improvements could be integrated in small steps. If the steps are smaller, you can focus much better on the single problem.

Would it be OK to open an issue for "not using a hardcoded id for the plugin" in the case this pr get merged? And we can work later on it?

That's a decision to be taken by 4.0 production leadership. ?

@wilsonge
I think you are production leadership for 4.0. What do you think concerning hardcoded id for plugins. Could we do it in an separate issue. By the way: There are many hard coded ids for plugins in the folder administrator/components/com_admin/sql/updates/mysql/... . This is not an argument for accuracy. But we could all correct it together.

@astridx do you think it should be a warning and not a notice when it says "x overridden files have been updated" ??

@brianteeman
No, I think the notice is good. It does not have to be that the override should be changed. At the moment we ignored it when the original to an override changes. It should only be an indication that something has changed. So no warning only a notice - in my opinion.

Perhaps you meant my comment "Only the messages could be improved" in the last picture of #21851 (comment). I meant the missing language string.

@astridx
Concerning the sql, I do not think it is an issue. When 3.9/3/10 are merged into 4.0, whoever merges should take care of it.

@astridx no you understood me correctly although I am not sure i agree- and I will review all the strings later today

@brianteeman Please take a look at these strings and suggest good if possible.

1. Added to list - -> When file history created
2. Last change by update --> When in next update that file changed again then it should update.
3. Delete list entry -> For delete of entry.

@brianteeman You asked:

Is there a reason that the diff view doesnt show line numbers?

We decided to use https://github.com/kpdecker/jsdiff for the diff view.
The reason for this was, that we do not need to integrate a new vendor script. jsdiff is already in Joomla. We use jsdiff for content history.

But jsdiff does not provide a line number. We would have to expand this ourselves.
@Anu1601CS Please correct me, if I am wrong.

When the quickicon is enabled but the override check is not enabled you get a nice message (language string change coming) BUT the link doesnt work. Sorry can't hep with the code but we have other places such as com_finder with similar that you should check

Would be good if somebody can open an issue for the hardcoded id. Just that it doesn't get forgotten.

@laoneo Shold we open this issue now? We are not sure if this PR get merged.

https://github.com/joomla-projects/gsoc18_override_management/blob/a33915f8d99dd945de5248e1bb94dc7c4deb0a17/administrator/components/com_templates/tmpl/template/default.php#L117

Didn't check the purpose of this if statement but it is preventing the display of the title

@astridx I am not sure if I will have time for such detailled tests as you did. And my plan was to wait with tests until @brianteeman has reviewed the language strings.

This is still to be fixed
#21851 (comment)

Rename and delete file - even though not selected they are ??
Should work the same as "delete list entry"

Can't replicate anymore

Is "Delete List Entry" the same as Marking it as Checked

COM_TEMPLATES_N_OVERRIDE_CHECKED
COM_TEMPLATES_N_OVERRIDE_DELETED
COM_TEMPLATES_N_OVERRIDE_UNCHECKED

These should all be changed to
COM_TEMPLATES_N_OVERRIDE_CHECKED_1
COM_TEMPLATES_N_OVERRIDE_CHECKED_MORE

See COM_CHECKIN_N_ITEMS_CHECKED_IN_1 as an example for the code to change

The language strings should then be updated to something like
COM_TEMPLATES_N_OVERRIDE_CHECKED_1="1 overridden file checked."
COM_TEMPLATES_N_OVERRIDE_CHECKED_MORE="%d overridden files checked."

@brianteem No, if you mark it as checked you do not see the notice any more. But it is not deleted. You can uncheck it again.
It is similar to disable an item.

So there needs to be a Checked and Unchecked button in the toolbar in that case

@astridx The issue with hard-coded extension ID war here:
File build/media_src/plg_quickicon_overridecheck/js/overridecheck.es6.js, line 51

This is not the same as using hard-coded IDs in schema update SQL scripts, it is a JS here.

Does anybody have an idea how to change that?

(Submitted some PR to the GSOC branch - logging off now)

I've submitted a new issue to the gsoc18_override_management repo as a reminder for the hard-coded extension ID in the Javascript file.

https://github.com/joomla-projects/gsoc18_override_management/blob/a33915f8d99dd945de5248e1bb94dc7c4deb0a17/administrator/components/com_templates/tmpl/template/default.php#L117

Didn't check the purpose of this if statement but it is preventing the display of the title

@brianteeman currently if you open file in jommla 4.0-dev then you can't see any title. So, that's why I added if override file got selected then I am showing titles.

ok that makes sense but its not working

@brianteeman Here (Linux environment) the title display for the override file works.

@brianteeman Can it be related to you having a Windows environment and to the backslash problem you found in the paths in the list of changed files?
When $overrideCheck['1'] is equal to html\ and not to html, that could be an explanation why on Windows the title is not shown because the check of the if condition fails.

yes most likely

But then $this->source->filename already would be wrong on Windows, and then I have no idea why other things are working or if it is maybe a core problem not related to this PR.

@brianteeman I've created a PR in the project repo for the issues on Windows:
joomla-projects/gsoc18_override_management#84.
Can you test?

@brianteeman In which case its not working #21851 (comment)

May be same windows or Linux problem

@Anu1601CS Brian and me already discussed that above. It is a problem only on Windows, and that was what my PR against the project repository was about.

More changes "core file" to "original file" in language strings proposed as PR to the project repo:
joomla-projects/gsoc18_override_management#85

@brianteeman @richard67 Check this commit this will remove hardcoded plugin id. i hope this will work for you.
ca8dc67

Late to the party here but #20500 is already open regarding the list of hardcoded IDs. Though similar to other RFCs I doubt it's going anywhere.

@Anu1601CS Commit ca8dc67 looks ok to me.

@brianteeman @richard67 Check this commit for non mysql 7eb7ff1

@Anu1601CS

Check this commit for non mysql 7eb7ff1

I guess you meant "Windows platform" and not "non mysql". That commit has nothing to do with database type.

Not sure if that is the right place to fix it and if it will help with display of the changed files list.

With the missing title it will help.

Let's see if Brian is ok with it, and after you merged the project branch into here so we have here the latest language change I can test again tomorrow evening (central european time).

@Anu1601CS Forget my previous comment, was about the wrong thing.

@zero-24 The issue tracker seems to have a problem with this PR. @astridx 's test is still shown as successful even after new commits have been made, and her test is shown with the wrong commit number b93b118, which was long time before many later commits and then her test.

Do you know if this can be fixed? Maybe this PR has so many commits that it confuses the issue tracker?

I will test soon, too, and am almost sure my test will be shown with the same old commit number b93b118.

@brianteeman @richard67 Yes, this PR has too many commits. I would suggest you to merge this PR after next successful tests. And if any issue arises then submit the issue here https://github.com/joomla-projects/gsoc18_override_management/issues i will do my PR here.

What do you think @astridx @laoneo @wilsonge @zero-24 ?

I have tested this item ✅ successfully on b93b118

Tested as described here and here.

In opposite to what the generated test message above sais, I tested on last commit 1468d4e9251f203451b923483278778b57357f45. It seems the issue tracker has a problem to follow the commit history of this PR.

Thanks @astridx for the testing instructions and @zero-24 for the custom Joomla update URL.

@astridx Can you test again or reset your test result? The issue tracker has some problems to notice that new commits have been made after a test.

And if any issue arises then submit the issue here https://github.com/joomla-projects/gsoc18_override_management/issues i will do my PR here.

I think when the PR is merged any new issue should be raised in the main repo. Merge decision is done by @wilsonge for 4.0 so he need to take a decision here.

I have repeated my tests (https://issues.joomla.org/tracker/joomla-cms/21851#event-388645 and https://issues.joomla.org/tracker/joomla-cms/21851#event-388707). The essentials were successful. So I think I do not have to change something in issue tracker, right?

I found something else to change. (I want to create a PR, but it was not possible for me. I did not see your local repo and branch as a base fork and I do not know how to achieve that.).
Please delete line 184 and change line 185 (https://github.com/joomla/joomla-cms/pull/21851/files#diff-ae4af84085ee35437e316b7653830bc0R184) to
$this->app->enqueueMessage(Text::plural('PLG_INSTALLER_N_OVERRIDE_FILE_UPDATED', $num), 'notice');

@astridx If you want to make a PR against the branch of this PR you just enter the URL of the repo in the adress bar of your browser: https://github.com/Anu1601CS/joomla-cms/. Then you can select the gsoc branch and make a PR. Selecting that repo as base in the compare view of the joomla/joomla-cms repo does indeed not work.

@Anu1601CS There is still the change from the last comment by Quy (see here) to be done, i.e. change the comment from "Trigger event before joomla." to "Trigger event before Joomla update." . Beside this I suggest that you check again also other comments and change "joomla" to "Joomla", because "Joomla" is a name and so starting with uppercase letter.

@richard67 See this commit 0abf606

@richard67 Thank you very much for explaining. But I did this. Nevertheless I was not able to select the gsoc branch from Anu1601CS.

@Anu1601CS Oh, seems I missed that. Sorry. All ok.

I found something else to change. (I want to create a PR, but it was not possible for me. I did not see your local repo and branch as a base fork and I do not know how to achieve that.).
Please delete line 184 and change line 185 (https://github.com/joomla/joomla-cms/pull/21851/files#diff-ae4af84085ee35437e316b7653830bc0R184) to
$this->app->enqueueMessage(Text::plural('PLG_INSTALLER_N_OVERRIDE_FILE_UPDATED', $num), 'notice');

@astridx
Current condition is like this

but according to your suggested changes it looks liks

So, i thought to make number background white to make number clear and good.

@astridx As I wrote you have to be in Anu1601CS's repository. But your screenshot shows you are in the core (joomla) repository. There is it normal that some user repositories are not found for selecting the base.
But if you go to https://github.com/Anu1601CS/joomla-cms/, then select there the gsoc branch, and then navigate to the file and edit it in the browser it works.
.

To the maintainers: My test is still valid because since then only CS changes.

To the maintainers: My test is still valid because since then only CS changes.

@Anu1601CS
I think you need to put the html for making the number nice into the language string. If I am right, you need to use a number as second parameter in the function plural:

I did comment earlier about switching to plural strings - the markup for styling the number should not however go in the language string if at all possible

@brianteeman Sorry, I could not find your earlier comment. So it is OK use a string as second parameter of this function?

@astridx @brianteeman 's comment is there: #21851 (comment). Is hard to find because initially hidden by GitHub because of very long comment history of this PR.

@richard67
Thank you very much.
First: I have to take a closer look to creating a PR against the branch of another PR.
Second: I searched for "plural". So I could not find it via search.

@brianteeman In this comment you have suggested using plural, right? The function plural have to be given a number as a second parameter. How can we add html, if we should not do it via language string? I found no example ...

Please do not add any html in a plural string.
It is perfectly OK if the number, which can be literal in the case of 1, i.e. for example in French

COM_REDIRECT_N_LINKS_UPDATED="%d liens mis à jour."
COM_REDIRECT_N_LINKS_UPDATED_1="Un lien a été mis à jour"

is not specifically formatted as it is anyway in a phrase.
It would make it more complex than necessary.

Note: the way you did it "would" be the way to go, but I am not sure we should anyway have html in that kind of file.

$span = '<span class="badge badge-light">' . $num . '</span>';
$this->app->enqueueMessage(Text::plural('PLG_INSTALLER_N_OVERRIDE_FILE_UPDATED', $span), 'notice');

I would say, that after the language strings cleanup is done. We merge it and then we can finalize further issues directly in this repo. Or are there any more blockers detected?

@laoneo agreed

To the maintainers: My test is still valid because since then only one change that I have suggested have occurred. At the time, I also tested these changes.

Thank you very much @Anu1601CS and @astridx. You guys did an awesome job!! And of course to the whole GosC team who helped whenever we have been struggling.

Thank you very much @astridx @laoneo @brianteeman @richard67 @zero-24 @Quy
For the review and reading such a huge code and finding my mistakes :)

small typo:
#22024