? Release Blocker NPM Resource Changed bug PR-4.3-dev Pending

User tests: Successful: Unsuccessful:

avatar dgrammatiko
dgrammatiko
16 Apr 2023

Pull Request for Issue #40393 .

Summary of Changes

Fix the logic

Testing Instructions

follow the issue

Actual result BEFORE applying this Pull Request

Expected result AFTER applying this Pull Request

This should be a release blocker

avatar joomla-cms-bot joomla-cms-bot - change - 16 Apr 2023
Category JavaScript Repository NPM Change
avatar dgrammatiko dgrammatiko - open - 16 Apr 2023
avatar dgrammatiko dgrammatiko - change - 16 Apr 2023
Status New Pending
avatar dgrammatiko dgrammatiko - change - 16 Apr 2023
Labels Added: NPM Resource Changed PR-4.3-dev
avatar Scrabble96
Scrabble96 - comment - 17 Apr 2023

I just tried to test this using the Patch Tester component and got this message:

"There are no files to patch from this pull request. This may mean that the files in the pull request are not present in your installation."

My overrides are in a custom template, not Cassiopeia

avatar brianteeman
brianteeman - comment - 17 Apr 2023

you can not use patch tester for this type of pull request.

avatar richard67 richard67 - test_item - 17 Apr 2023 - Tested successfully
avatar richard67
richard67 - comment - 17 Apr 2023

I have tested this item successfully on 04eac35

I've tested as described in my comment to the issue here: #40393 (comment)


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

avatar MacJoom MacJoom - test_item - 17 Apr 2023 - Tested successfully
avatar MacJoom
MacJoom - comment - 17 Apr 2023

I have tested this item successfully on 04eac35

After applying the patch and rebuilding javascript the check button worked. Thanks to richard for providing the sql commands to simulate an overide in the template


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

avatar richard67 richard67 - change - 17 Apr 2023
Status Pending Ready to Commit
avatar richard67
richard67 - comment - 17 Apr 2023

RTC


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

avatar MacJoom
MacJoom - comment - 17 Apr 2023

you can not use patch tester for this type of pull request.

You can use patch tester for PR's with the tag "NPM Resource changed" only if you have setup a local environment for Joomla as described here: https://docs.joomla.org/J4.x:Setting_Up_Your_Local_Environment/en - then the file(s) to be patched exist
and you need to do npm ci - or even jinstall after the patch

avatar richard67
richard67 - comment - 17 Apr 2023

@MacJoom You still need to run npm to compile the js after having applied the PR.

avatar MacJoom
MacJoom - comment - 17 Apr 2023

@MacJoom You still need to run npm to compile the js after having applied the PR.

just updated my comment...

avatar brianteeman
brianteeman - comment - 17 Apr 2023

you can not use patch tester for this type of pull request.

You can use patch tester for PR's with the tag "NPM Resource changed" only if you have setup a local environment for Joomla as described here: https://docs.joomla.org/J4.x:Setting_Up_Your_Local_Environment/en - then the file(s) to be patched exist

You really can't. People using patchtester expect to apply the patch and then be able to test - even with the files in their environment there is still a lot more to do. Far better to tell them the correc5t advice as provided by @richard67

avatar brianteeman
brianteeman - comment - 17 Apr 2023

or even jinstall after the patch

that's a new one???

avatar richard67
richard67 - comment - 17 Apr 2023

or even jinstall after the patch

that's a new one???

@brianteeman I think he means making a new installation of Joomla e.g. when a PR has changed in SQL scripts for new installations.

avatar MacJoom
MacJoom - comment - 17 Apr 2023

or even jinstall after the patch

that's a new one???

Not really it's described in https://docs.joomla.org/J4.x:Setting_Up_Your_Local_Environment/en - a batch command that does composer install and npm ci

avatar brianteeman
brianteeman - comment - 17 Apr 2023

seriously? ;)
if you are using patchtester then you almost certainly not in a position to create a bash alias script
a script which btw is hopelessy out of date

avatar obuisard obuisard - change - 25 Apr 2023
Labels Added: ? Release Blocker bug
avatar obuisard obuisard - change - 25 Apr 2023
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2023-04-25 14:20:22
Closed_By obuisard
avatar obuisard obuisard - close - 25 Apr 2023
avatar obuisard obuisard - merge - 25 Apr 2023
avatar obuisard
obuisard - comment - 25 Apr 2023

Thanks Dimitris @dgrammatiko for the fix and all for testing!

Add a Comment

Login with GitHub to post a comment