Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[4.4] Fix newsfeed category html view #43879

Merged
merged 10 commits into from
Aug 6, 2024

Conversation

muhme
Copy link
Contributor

@muhme muhme commented Aug 4, 2024

Summary of Changes

Fix newsfeed category html view as down-merge from 5.1-dev. This is a sub-part from #43722 to use joomla-cypress 1.1.1 in 4.4-dev too.

Testing Instructions

The warning is displayed in A) the frontenend view and B) the System Test (with a speaking checkForPhpNoticesOrWarnings). Verify the warning before patching. After patching, check that there are no more warnings.

Actual result BEFORE applying this Pull Request

A) Manual Test

Requesting URL /index.php?option=com_newsfeeds&view=category&id=5 in fresh 4.4-dev shows the PHP Warning message (id comes from Components | News Feeds | Categories | Uncategorised):
shoot

B) System Test

As preparation install speaking checkForPhpNoticesOrWarnings in overwriting lines 74 ... 89 in node_modules/joomla-cypress/src/support.jswith the following lines:

  Cypress.Commands.add('checkForPhpNoticesOrWarnings', () => {
    cy.log('**Check for PHP notices and warnings**')

    cy.document().then((doc) => {
      const pageSource = doc.documentElement.innerHTML
      // Search for PHP problem keywords in bold style with colon and collect the found keyword and the message
      const regex = /<b>(Warning|Deprecated|Notice|Strict standards)<\/b>:(.*?)(<br|$)/
      const match = regex.exec(pageSource)
      if (match) {
        // Directly fail with the reason, the keyword found and report the PHP problem message, e.g.
        // Error: Unwanted PHP Warning: "Attempt to read property \"id\" on null in <b>/joomla-cms/components/com_newsfeeds/src/View/Category/HtmlView.php</b> on line <b>92</b>"
        throw new Error(`Unwanted PHP ${match?.[1]}: ${JSON.stringify(match?.[2])}`)
      }
    })

    cy.log('--Check for PHP notices and warnings--')
  })

Run the test specifications:

npx cypress run --spec tests/System/integration/site/components/com_newsfeed/Category.cy.js

Fails with:

  Running:  Category.cy.js                                                                  (1 of 1)

  Test in frontend that the newsfeeds category view
    ✓ can display a list of feeds in a menu item (539ms)
    1) "after each" hook for "can display a list of feeds without a menu item"

  1 passing (1s)
  1 failing

  1) Test in frontend that the newsfeeds category view
       "after each" hook for "can display a list of feeds without a menu item":
     Error: Unwanted PHP Warning: "  Attempt to read property \"id\" on null in <b>/Users/hlu/Desktop/no_backup/joomla-cms/components/com_newsfeeds/src/View/Category/HtmlView.php</b> on line <b>92</b>"

💡 With overwritten speaking checkForPhpNoticesOrWarnings, if you run the entire system tests, there are four failures before this PR and only three after. The remaining three errors are fixed by #43880 and #43881.

Expected result AFTER applying this Pull Request

No warnings in A) frontend view or B) System Test.

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

Up-Merging

@LadySolveig @bembelimen there is no up-merge required as it is only a down-merge from 5.1-dev

muhme added 9 commits June 7, 2024 06:47
Consider file permissions when writing configuration in system tests …
merge joomla/joomla-cms branch 4.4-dev
merge head-repository joomla/joomla-cms
merge head repository joomla/joomla-cms
merge from head repository
@alikon
Copy link
Contributor

alikon commented Aug 5, 2024

I have tested this item ✅ successfully on fe9bd06


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

@muhme
Copy link
Contributor Author

muhme commented Aug 5, 2024

I have tested this item ✅ successfully on fe9bd06

To warm up with Joomla! Patch Tester, I tested my own PR with A) Manual Test on local 4.4.7-dev, frontend URL /index.php?option=com_newsfeeds&view=category&id=5 shows "Warning: Attempt to read property "id" on null in /Users/hlu/Desktop/no_backup/joomla-cms/components/com_newsfeeds/src/View/Category/HtmlView.php on line 92" After applying the PR the warning is gone.


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

@fgsw
Copy link

fgsw commented Aug 6, 2024

I have tested this item ✅ successfully on fe9bd06

Manual Test (A) only.


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

@laoneo laoneo removed the PR-4.4-dev label Aug 6, 2024
@laoneo
Copy link
Member

laoneo commented Aug 6, 2024

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Aug 6, 2024
@laoneo laoneo merged commit 23afb6d into joomla:4.4-dev Aug 6, 2024
3 checks passed
@laoneo
Copy link
Member

laoneo commented Aug 6, 2024

Thanks!

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Aug 6, 2024
@laoneo laoneo added this to the Joomla! 4.4.7 milestone Aug 6, 2024
@muhme muhme deleted the fix-newsfeed-category-htmlview branch August 6, 2024 08:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants