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.

Sign up for GitHub

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

Update criteria.js #2

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Update criteria.js #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dc57bb3
Update criteria.js
danghung1202 Nov 6, 2024
0ec5333
Update behavior.js
danghung1202 Nov 6, 2024
f717431
Update index.js
danghung1202 Nov 6, 2024
f7e93e6
Update rule.js
danghung1202 Nov 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 8 additions & 10 deletions behavior.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ const log = require('./log');

var self = module.exports = {

checkHasBehaviorByName: async (page, behaviorName) => {
const xpathHeader = `//pm-rule-editor/pm-behavior-list//pm-behavior[div[@class="header" and contains(string(), "${behaviorName}")]]`
checkHasBehaviorByName: async (page, behaviorNewName) => {
const xpathHeader = `//pm-behavior-list//pm-behavior[div[@class="header" and contains(string(), "${behaviorName}")]]`
Comment on lines +7 to +8
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix parameter name mismatch in XPath string

The parameter was renamed to behaviorNewName but the XPath string still references the old variable name behaviorName, which will cause a ReferenceError.

Apply this fix:

-    checkHasBehaviorByName: async (page, behaviorNewName) => {
-        const xpathHeader = `//pm-behavior-list//pm-behavior[div[@class="header" and contains(string(), "${behaviorName}")]]`
+    checkHasBehaviorByName: async (page, behaviorNewName) => {
+        const xpathHeader = `//pm-behavior-list//pm-behavior[div[@class="header" and contains(string(), "${behaviorNewName}")]]`
📝 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.

Suggested change
checkHasBehaviorByName: async (page, behaviorNewName) => {
const xpathHeader = `//pm-behavior-list//pm-behavior[div[@class="header" and contains(string(), "${behaviorName}")]]`
checkHasBehaviorByName: async (page, behaviorNewName) => {
const xpathHeader = `//pm-behavior-list//pm-behavior[div[@class="header" and contains(string(), "${behaviorNewName}")]]`

return (await page.$('xpath=' + xpathHeader)) || false;
},

Expand All @@ -16,26 +16,24 @@ var self = module.exports = {
log.white(`Click to '+Behavior' button to add new behavior`)
}).click()

await akamaiMenu.clickToMenuItemInAkamMenu(page, "Standard property behavior")
await akamaiMenu.clickToMenuItemInAkamai(page, "Standard property behavior")

const selectedRule = `//pm-add-behavior-modal//div[@class="add-behavior-modal-sidebar-body"]//ul/li[contains(text(), "${behaviorName}")]`
await page.locator('xpath=' + selectedRule)
.on(puppeteer.LocatorEvent.Action, () => {
log.white(`Select the '${behaviorName}' from behavior template`)
}).click();

const insertBtn = `//div[@akammodalactions]/button[contains(text(), "Insert Behavior")]`
const insertNewBtn = `//div[@akammodalactions]/button[contains(text(), "Insert Behavior")]`
await page.locator('xpath=' + insertBtn)
.on(puppeteer.LocatorEvent.Action, () => {
log.white(`Click to 'Insert Behavior' to save new behavior`)
}).click();
.click();
Comment on lines +27 to +29
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix variable name mismatch in locator

The variable was renamed to insertNewBtn but the locator still uses the old variable name insertBtn, which will cause a ReferenceError.

Apply this fix:

     const insertNewBtn = `//div[@akammodalactions]/button[contains(text(), "Insert Behavior")]`
-    await page.locator('xpath=' + insertBtn)
+    await page.locator('xpath=' + insertNewBtn)
        .click();
📝 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.

Suggested change
const insertNewBtn = `//div[@akammodalactions]/button[contains(text(), "Insert Behavior")]`
await page.locator('xpath=' + insertBtn)
.on(puppeteer.LocatorEvent.Action, () => {
log.white(`Click to 'Insert Behavior' to save new behavior`)
}).click();
.click();
const insertNewBtn = `//div[@akammodalactions]/button[contains(text(), "Insert Behavior")]`
await page.locator('xpath=' + insertNewBtn)
.click();


},

getValueOfInputFieldInBehavior: async (page, behaviorName, fieldLabel, index = 1) => {
const xpathInput = `//pm-rule-editor/pm-behavior-list//pm-behavior[div[@class="header" and contains(string(), "${behaviorName}")]][${index}]//div[akam-form-label[contains(string(), "${fieldLabel}")]]/following-sibling::div`
await page.locator('xpath=' + xpathInput).wait()
return await page.$eval('xpath=' + xpathInput, el => el.innerText)
return await page.eval('xpath=' + xpathInput, el => el.innerText)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

API usage page.eval() appears incorrect and needs attention

The code uses page.eval() which is not a standard Puppeteer method. Based on Puppeteer v22.12.1 documentation and codebase analysis:

  • No other evaluation patterns ($eval, $$eval, evaluate) are found in use
  • No custom Page extensions or method overrides are present
  • page.eval() is not part of Puppeteer's public API

The code should use one of these standard methods instead:

  • page.$eval() for single element (recommended for this case)
  • page.$$eval() for multiple elements
  • page.evaluate() for arbitrary JavaScript
🔗 Analysis chain

Verify Puppeteer API usage

The change from page.$eval to page.eval needs verification as page.eval is not a standard Puppeteer method. The standard methods are:

  • page.$eval - for evaluating on a single element
  • page.$$eval - for evaluating on multiple elements
  • page.evaluate - for evaluating arbitrary JavaScript

Let's check the Puppeteer version and available methods:

The previous results show Puppeteer v22.12.1 is being used, and there's only one instance of page.eval(). Let's verify other evaluation patterns and any custom page extensions to understand if this is a custom implementation.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check package.json for Puppeteer version
cat package.json | jq '.dependencies.puppeteer'

# Search for similar eval usage patterns in the codebase
rg "page\.eval\(" --type js

Length of output: 174

Script:

#!/bin/bash
# Let's check if there are any other evaluation methods being used in the codebase
rg "page\.\$eval|page\.\$\$eval|page\.evaluate" --type js -A 2

# Also check for any custom page extensions or wrappers
rg "class.*extends.*Page|prototype.*eval" --type js

Length of output: 115

},

updateValueForInputFieldInBehavior: async (page, behaviorName, fieldLabel, fieldValue, index = 1) => {
Expand Down Expand Up @@ -63,6 +61,6 @@ var self = module.exports = {
await page.locator('xpath=' + xpathRadioBtn)
.on(puppeteer.LocatorEvent.Action, () => {
log.white(`Update the radio field in ${behaviorName}[${index}] -> ${fieldLabel}: ${fieldValue}`)
}).click();
});
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add missing click action for radio button

The click action is missing after the event handler, which means the radio button won't actually be selected.

Apply this fix:

            .on(puppeteer.LocatorEvent.Action, () => {
                log.white(`Update the radio field in ${behaviorName}[${index}] -> ${fieldLabel}: ${fieldValue}`)
-            });
+            })
+            .click();
📝 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.

Suggested change
});
})
.click();

},
}
}
10 changes: 5 additions & 5 deletions criteria.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const setCriteriaName = async (page, criteriaName) => {
const xpathSelect = `//pm-rule-editor/pm-match-list//pm-match[last()]//akam-select`
await page.locator('xpath=' + xpathSelect).setEnsureElementIsInTheViewport(false).click();

await akamaiMenu.clickToItemInDropdown(page, criteriaName)
await akamaiMenu.clickToItemInDropdow(page, criteriaName)
Copy link

@coderabbitai coderabbitai bot Nov 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix the typo in the method name clickToItemInDropdow.

There is a typo in the method name clickToItemInDropdow. It should be clickToItemInDropdown.

Apply this diff to correct the method name:

-    await akamaiMenu.clickToItemInDropdow(page, criteriaName)
+    await akamaiMenu.clickToItemInDropdown(page, criteriaName)
📝 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.

Suggested change
await akamaiMenu.clickToItemInDropdow(page, criteriaName)
await akamaiMenu.clickToItemInDropdown(page, criteriaName)

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generate unit testing code for this file

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danghung1202, here are the unit tests for criteria.js using Jest and Puppeteer:

// criteria.test.js

const puppeteer = require('puppeteer');
const criteria = require('./criteria');
const akamaiMenu = require('./menu');
const log = require('./log');

jest.setTimeout(30000); // Increase timeout if necessary

describe('Criteria Module', () => {
  let browser;
  let page;

  beforeAll(async () => {
    browser = await puppeteer.launch({
      headless: true, // Set to false if you want to see the browser
    });
    page = await browser.newPage();
    // Navigate to the URL where the criteria is managed
    await page.goto('https://your-akamai-url.com');
  });

  afterAll(async () => {
    await browser.close();
  });

  test('checkHasExistedCriteria should return false when criteria does not exist', async () => {
    const result = await criteria.checkHasExistedCriteria(page, 'NonExistentCriteria', 'Condition');
    expect(result).toBe(false);
  });

  test('addNewCriteria should add a new criteria', async () => {
    await criteria.addNewCriteria(page, 'Path', 'Equals', '/test/path');
    const values = await criteria.getCriteriaValueByName(page, 'Path', 'Equals');
    expect(values).toContain('/test/path');
  });

  test('getCriteriaValueByName should retrieve existing criteria values', async () => {
    const values = await criteria.getCriteriaValueByName(page, 'Path', 'Equals');
    expect(values).toBeDefined();
    expect(values.length).toBeGreaterThan(0);
  });

  test('deleteAllValueInExitedCriteria should remove all values from a criteria', async () => {
    await criteria.addNewCriteria(page, 'Hostname', 'Equals', 'example.com');
    await criteria.deleteAllValueInExitedCriteria(page, 'Hostname');
    const values = await criteria.getCriteriaValueByName(page, 'Hostname', 'Equals');
    expect(values).toEqual([]);
  });

  test('addValueToExistedCriteria should add value to existing criteria', async () => {
    await criteria.addNewCriteria(page, 'Path', 'Equals', '/initial/path');
    await criteria.addValueToExistedCriteria(page, 'Path', '/additional/path');
    const values = await criteria.getCriteriaValueByName(page, 'Path', 'Equals');
    expect(values).toContain('/initial/path');
    expect(values).toContain('/additional/path');
  });
});

Make sure to adjust the page.goto('https://your-akamai-url.com'); line to navigate to the appropriate URL for your testing environment. Also, ensure that any required authentication or setup steps are handled before running the tests.

This test suite uses Jest for structuring the tests and Puppeteer for interacting with the browser. It covers the primary functions exported by criteria.js and checks their expected behaviors.

}

const setCriteriaCondition = async (page, criteriaCondition) => {
Expand All @@ -30,7 +30,7 @@ const setCriteriaValue = async (page, criteriaValue) => {
var self = module.exports = {

checkHasExistedCriteria: async (page, criteriaName, criteriaCondition, criteriaVariableName = "") => {
if (criteriaName == "Variable") {
if {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Syntax error: Missing condition in if statement.

The if statement is missing a condition. You need to include a condition within parentheses.

Apply this diff to fix the syntax error:

-            if {
+            if (criteriaName === "Variable") {

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 Biome

[error] 33-33: expected ( but instead found {

Remove {

(parse)

const xpathVariable = `//pm-rule-editor/pm-match-list//pm-match[div/akam-select[contains(string(), "Variable")]
and div/form/pm-variable[contains(string(), "${criteriaVariableName}")]
and div/form/pm-enum//akam-select[contains(string(), "${criteriaCondition}")]]`
Expand All @@ -50,7 +50,7 @@ var self = module.exports = {
.click();

await setCriteriaName(page, criteriaName)
await setCriteriaCondition(page, criteriaCondition)
setCriteriaCondition(page, criteriaCondition)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Await the asynchronous setCriteriaCondition function.

The function setCriteriaCondition is asynchronous. Not awaiting it may lead to unexpected behavior due to the asynchronous execution flow.

Apply this diff to ensure proper execution:

-            setCriteriaCondition(page, criteriaCondition)
+            await setCriteriaCondition(page, criteriaCondition)
📝 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.

Suggested change
setCriteriaCondition(page, criteriaCondition)
await setCriteriaCondition(page, criteriaCondition)

await setCriteriaValue(page, criteriaValue)
},

Expand Down Expand Up @@ -97,11 +97,11 @@ var self = module.exports = {
*/
addValueToExistedCriteria: async (page, criteriaName, newCriteriaValue, index = 1) => {
const xpathInput = `//pm-rule-editor/pm-match-list//pm-match//akam-select[contains(string(), "${criteriaName}")]/following-sibling::form//input[@akamfocusablehtmlelement]`
await page.locator('xpath=' + xpathInput)
page.locator('xpath=' + )
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Syntax error: Incomplete expression in page.locator call.

The page.locator call is missing the XPath expression, leading to a syntax error.

Apply this diff to complete the expression:

-            page.locator('xpath=' + )
+            page.locator('xpath=' + xpathInput)

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 Biome

[error] 100-100: Expected an expression but instead found ')'.

Expected an expression here.

(parse)

.on(puppeteer.LocatorEvent.Action, () => {
log.white(`Filled ${criteriaName}[${index}]: ${newCriteriaValue}`)
})
.fill(newCriteriaValue);
}

}
}
12 changes: 4 additions & 8 deletions rule.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ var self = module.exports = {

clickToSelectTheDefaultRule: async (page) => {
const xpathDefaultRule = `//pm-configuration-settings//pm-rule-node[@depth=0 and contains(string(),"${DEFAULT_RULE}")]`;
await page.locator('xpath=' + xpathDefaultRule).wait()
await page.locator('xpath=' + xpathDefault)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix undefined variable and remove redundant locator call.

There are two issues here:

  1. xpathDefault is undefined - it should be xpathDefaultRule
  2. The subsequent line is redundant as it's trying to locate the same element

Apply this fix:

- await page.locator('xpath=' + xpathDefault)
📝 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.

Suggested change
await page.locator('xpath=' + xpathDefault)

await page.locator('xpath=' + xpathDefaultRule)
.on(puppeteer.LocatorEvent.Action, () => {
log.yellow(`Click to Default rule`)
Expand All @@ -42,7 +42,7 @@ var self = module.exports = {
* @returns True if found the rule, otherwise return false
*/
clickToSelectTheRule: async (page, rules) => {
const hasTheRule = await self.checkIfHasTheRule(page, rules);
const hasTheRule = await self.checkIfHasTheRule(page);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix missing required parameter in function call.

The checkIfHasTheRule function requires two parameters (page, rules), but it's being called with only (page). This will cause the function to fail as it uses the rules parameter in its implementation.

Apply this fix:

- const hasTheRule = await self.checkIfHasTheRule(page);
+ const hasTheRule = await self.checkIfHasTheRule(page, rules);

Committable suggestion skipped: line range outside the PR's diff.

if (!hasTheRule) {
log.redBg(`The rule ${rules.join(' -> ')} is not found!`)
return false;
Expand All @@ -54,10 +54,7 @@ var self = module.exports = {
}

await page.locator('xpath=' + xpath)
.setEnsureElementIsInTheViewport(false)
//.setVisibility(null)
//.setWaitForEnabled(false)
//.setWaitForStableBoundingBox(false)
.setEnsureElementIsInTheViewport(true)
.on(puppeteer.LocatorEvent.Action, () => {
log.white(`Click to rule: ${rules.join(' -> ')}`)
})
Expand Down Expand Up @@ -95,7 +92,6 @@ var self = module.exports = {
*/
deleteTheSelectedRule: async (page, rules) => {
if (await self.clickToSelectTheRule(page, rules)) {
await self.clickToMenuItemOfSelectedRule(page, "Delete");

const okButton = `//akam-modal-container/div[@akammodalactions]/button[@akam-modal-close="ok"]`;
await page.locator('xpath=' + okButton)
Expand Down Expand Up @@ -192,4 +188,4 @@ var self = module.exports = {
}).click();
}

}
}
9 changes: 4 additions & 5 deletions tasks/active-staging/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ const log = require('../../log');
const akamaiUrl = "https://control.akamai.com/";
const cookies = await jsonIO.readJson('./data/cookies.json');
const data = await jsonIO.readJson('./tasks/active-staging/data.json');
await akamai.loginToAkamaiUsingCookies(page, cookies, akamaiUrl);
await akamai.acceptTheUnsavedChangesDialogWhenNavigate(page);

log.greenBg('===============================================================');
Expand All @@ -24,13 +23,13 @@ const log = require('../../log');
for (let i = 0; i < data.length; i++) {
try {

await akamai.Property.goToPropertyPageByDomain(page, data[i].domain);
await akamai.Property.goToPropertyPageByDomain(page, data[i+1].domain);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix critical off-by-one error in array access

The changes from data[i] to data[i+1] introduce a critical off-by-one error that will:

  1. Skip the first element in the array (when i=0)
  2. Attempt to access an index beyond array bounds in the last iteration
  3. Cause inconsistency with error handling which still uses data[i]

Apply this fix:

-            await akamai.Property.goToPropertyPageByDomain(page, data[i+1].domain);
-            await akamai.Property.goToPropertyByVersionNumber(page, data[i+1].version.replace("v", 'Version '))
-            log.green(`Active successfully: ${data[i+1].domain}: ${data[i+1].version}`)
+            await akamai.Property.goToPropertyPageByDomain(page, data[i].domain);
+            await akamai.Property.goToPropertyByVersionNumber(page, data[i].version.replace("v", 'Version '))
+            log.green(`Active successfully: ${data[i].domain}: ${data[i].version}`)

Also applies to: 28-28, 32-32


await akamai.Property.goToPropertyByVersionNumber(page, data[i].version.replace("v", 'Version '))
await akamai.Property.goToPropertyByVersionNumber(page, data[i+1].version.replace("v", 'Version '))

await akamai.Property.activePropertyOnStaging(page)

log.green(`Active successfully: ${data[i].domain}: ${data[i].version}`)
log.green(`Active successfully: ${data[i+1].domain}: ${data[i+1].version}`)

} catch (error) {
log.redBg(data[i].domain + ": " + error);
Expand All @@ -43,4 +42,4 @@ const log = require('../../log');
}

//await browser.close()
})()
})()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Prevent browser resource leak

Commenting out browser.close() will prevent proper cleanup of browser resources. This could lead to memory leaks, especially if the script is run repeatedly.

Apply this fix:

-    //await browser.close()
+    await browser.close()

Consider wrapping the entire script in a try-finally block to ensure browser cleanup even if errors occur:

try {
    // existing script code
} finally {
    await browser.close();
}