-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat: adding new forms opportunity and sending urls for scraping #649
base: main
Are you sure you want to change the base?
Conversation
siteId: auditData.siteId, | ||
auditId: auditData.id, | ||
runbook: 'https://adobe.sharepoint.com/:w:/s/AEM_Forms/EU_cqrV92jNIlz8q9gxGaOMBSRbcwT9FPpQX84bRKQ9Phw?e=Nw9ZRz', | ||
type: 'high-page-views-low-form-cta', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Low-form-ctr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
type: 'high-page-views-low-form-cta', | ||
origin: 'AUTOMATION', | ||
title: 'Form has low views but CTA page has high views', | ||
description: 'Form has low views but CTA page has high view', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: views
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should rewrite desc as
The page containing the form CTA has high views but low CTR for the form CTA
runbook: 'https://adobe.sharepoint.com/:w:/s/AEM_Forms/EU_cqrV92jNIlz8q9gxGaOMBSRbcwT9FPpQX84bRKQ9Phw?e=Nw9ZRz', | ||
type: 'high-page-views-low-form-cta', | ||
origin: 'AUTOMATION', | ||
title: 'Form has low views but CTA page has high views', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But conversion element has low CTR
log.info(`Triggering scrape for [${JSON.stringify(uniqueUrls, null, 2)}]`); | ||
if (uniqueUrls.size > 0) { | ||
await sqs.sendMessage(process.env.SCRAPING_JOBS_QUEUE_URL, { | ||
processingType: 'default', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ProcessingType : form
} | ||
|
||
const { formVitals } = auditData.auditResult; | ||
const formOpportunities = generateOpptyDataForHighPageViewsLowFormCTA(formVitals); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LowFormCTR
* @param auditData - The audit data containing the audit result and additional details. | ||
* @param context - The context object containing the data access and logger objects. | ||
*/ | ||
export default async function highPageViewsLowFormCTAOpportunity(auditUrl, auditData, context) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LowFormCTR
export default async function highPageViewsLowFormCTAOpportunity(auditUrl, auditData, context) { | ||
const { dataAccess, log } = context; | ||
const { Opportunity } = dataAccess; | ||
log.info(`Syncing high page views low form cta opportunity for ${auditData.siteId}`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CTR
|
||
try { | ||
const opportunities = await Opportunity.allBySiteIdAndStatus(auditData.siteId, 'NEW'); | ||
highPageViewsLowFormCtaOppty = opportunities.find((oppty) => oppty.getType() === 'high-page-views-low-form-cta'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
low-form-ctr
const opportunityData = { | ||
siteId: auditData.siteId, | ||
auditId: auditData.id, | ||
runbook: 'https://adobe.sharepoint.com/:w:/s/AEM_Forms/EU_cqrV92jNIlz8q9gxGaOMBSRbcwT9FPpQX84bRKQ9Phw?e=Nw9ZRz', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you pls create a unique runbook for this besides the existing one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure will do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
const uniqueUrls = new Set(); | ||
formOpportunities.forEach((opptyData) => { | ||
uniqueUrls.add(opptyData.url); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This url should be of the page containing CTA
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inline comments
src/forms-opportunities/utils.js
Outdated
); | ||
|
||
// const formVitalsByDevice = aggregateFormVitalsByDevice(formVitalsCollection); | ||
return getHighPageViewsLowFormCtrMetrics(formVitalsCollection, 7).map(convertToOpportunityData); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oppoty data would need some changes for this oppoty type.
CtaPageUrl
MaxCTR
formCTR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oppty data for high page views low form ctr has
{
url: 'https://www.surest.com/newsletter',
pageViews: 8670,
formViews: 300,
formEngagement: 300,
CTA: {
url: 'https://www.surest.com/about-us',
source: '#teaser-related02 .cmp-teaser__action-link',
},
}
This PR will trigger a minor release when merged. |
Please ensure your pull request adheres to the following guidelines:
Related Issues
Thanks for contributing!