add-ons page validations#1963
Conversation
WalkthroughA new test file, Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@lauramekaj1 Does this update validate the old add-ons page we're replacing soon? Or does it work with the new add-ons update as well? The page is changing quite a bit so I'm concerned these tests could immediately break. The add-ons update is in this PR #1802 Have you run these tests against that branch? |
|
@Crabcyborg Yes, it validates the old page only. I tried to run the tests against the new add-ons PR #1802 but the tests fail. Do you think I should update my tests based on the PR #1802 and as soon as the new add-ons will be released then we can add our tests as well? |
|
@lauramekaj1 Yeah, I think we could branch off of Sherv's branch and merge it even into his branch ahead of time possibly. We're likely going to merge his update some time this week. And the release will happen on October 2 if it goes to schedule. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
Hi @Crabcyborg, I updated it based on the new add-ons. If everything looks ok then we can merge against the new-add-ons branch. Also, do you think it's better if I create a branch out of the new-add-ons branch and then will push the changes there? |
|
@Crabcyborg I think the search function is based on the pattern e.g. data-slug="signature". For example, when I try to search for "Digital signatures," no results are shown. However, when I search for just "signature," the search results are displayed. |
|
Thanks @lauramekaj1. I tried to come up with a solution to the plural search issue. I have a new pull request here #2007 Could you help test it? I assigned you for review. |
@lauramekaj1 That's how I would probably do it. If we just merge the add-ons branch into this one and change the base merge branch here in GitHub to the add-ons branch it should be fine. We wouldn't merge this before the other update. |
…idate_add_ons_page
@Crabcyborg I merged new-add-ons branch into this one and also changed the base merge. There are some tests which are failing here. I think it's because that the Cypress check is being run on master! |
Crabcyborg
left a comment
Crabcyborg
left a comment
There was a problem hiding this comment.
@lauramekaj1 I think the only issue was a little mistake with the search checks. It checks for the signatures add-on, but still had the validation for the PayPal check before that.
I made a small commit and it's passing now ce26573
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
tests/cypress/e2e/Add-Ons/validateAddOnsPage.cy.js (1)
110-111: Avoid ManipulatingtargetAttribute Unless NecessaryRemoving the
targetattribute before clicking may not be needed in this context. If the intention is to keep the navigation within the same browser tab during testing, this is acceptable. Otherwise, consider whether this step is necessary.If you decide to retain this step, adding a comment to clarify the reason could improve code readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- tests/cypress/e2e/Add-Ons/validateAddOnsPage.cy.js (1 hunks)
🔇 Additional comments (1)
tests/cypress/e2e/Add-Ons/validateAddOnsPage.cy.js (1)
14-18: Ensure Consistency in Text AssertionsThere is an inconsistency in the capitalization of "Pro" and "PRO" between the upgrade banner text and the link text. Verify that this difference is intentional and matches the actual content displayed on the page.
Run the following script to confirm the displayed texts:
| cy.get('#addon-search-input').clear().type("Add an electronic signature to your WordPress form. The visitor may write their signature with a trackpad/mouse or type it."); | ||
| cy.get('.plugin-card-signature').should("contain", "Digital Signatures"); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance Search Test with More Realistic Input
Searching using the entire description text may not reflect typical user behavior. Consider modifying the test to search using keywords or partial phrases to better simulate real-world usage and ensure the search functionality handles common scenarios effectively.
Apply this change to use a relevant keyword:
-cy.get('#addon-search-input').clear().type("Add an electronic signature to your WordPress form. The visitor may write their signature with a trackpad/mouse or type it.");
+cy.get('#addon-search-input').clear().type("electronic signature");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cy.get('#addon-search-input').clear().type("Add an electronic signature to your WordPress form. The visitor may write their signature with a trackpad/mouse or type it."); | |
| cy.get('.plugin-card-signature').should("contain", "Digital Signatures"); | |
| cy.get('#addon-search-input').clear().type("electronic signature"); | |
| cy.get('.plugin-card-signature').should("contain", "Digital Signatures"); |
| cy.contains('Plan required:').within(() => { | ||
| cy.get('a').should('have.attr', 'href', 'https://formidableforms.com/lite-upgrade/?utm_source=WordPress&utm_medium=addons&utm_campaign=liteplugin%20&%20utm_content%20=%20signature') | ||
| .and('contain.text', 'Business'); | ||
| }); |
There was a problem hiding this comment.
Validate URL Encoding in href Attributes
The href attribute contains URL-encoded spaces (%20), which may indicate unintended formatting. Ensure that the URLs are correctly encoded and that there are no extraneous spaces or encoding issues that could lead to incorrect links.
Apply this diff to correct the URL:
- cy.get('a').should('have.attr', 'href', 'https://formidableforms.com/lite-upgrade/?utm_source=WordPress&utm_medium=addons&utm_campaign=liteplugin%20&%20utm_content%20=%20signature')
+ cy.get('a').should('have.attr', 'href', 'https://formidableforms.com/lite-upgrade/?utm_source=WordPress&utm_medium=addons&utm_campaign=liteplugin&utm_content=signature')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cy.contains('Plan required:').within(() => { | |
| cy.get('a').should('have.attr', 'href', 'https://formidableforms.com/lite-upgrade/?utm_source=WordPress&utm_medium=addons&utm_campaign=liteplugin%20&%20utm_content%20=%20signature') | |
| .and('contain.text', 'Business'); | |
| }); | |
| cy.contains('Plan required:').within(() => { | |
| cy.get('a').should('have.attr', 'href', 'https://formidableforms.com/lite-upgrade/?utm_source=WordPress&utm_medium=addons&utm_campaign=liteplugin&utm_content=signature') | |
| .and('contain.text', 'Business'); | |
| }); |
2 participants
No description provided.