Skip to content

Conversation

@dzehnder
Copy link
Contributor

@dzehnder dzehnder commented Jan 30, 2024

API trigger implemented here: adobe/spacecat-api-service#130
Post Processor implemented here: adobe/spacecat-audit-post-processor#65

Copy link
Contributor

@iuliag iuliag left a comment

Choose a reason for hiding this comment

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

Looks good so far, with some observations and questions below

'lhs-desktop': lhs,
404: notfound,
'broken-backlinks': backlinks,
'organic-keywords': organicKeywords,
Copy link
Contributor

Choose a reason for hiding this comment

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

For all of the keys here, we could probably reuse the types exported from the audit model

],
};

return this.sendRequest('/site-explorer/organic-keywords', queryParams);
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the cost per row for this query?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Base const: 50
keyword: 1
best_position: 1
best_position_prev: 1
best_position_diff: 1
sum_traffic: 10

Total: 64 units

Should we document this somewhere?

Copy link
Contributor

Choose a reason for hiding this comment

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

Good idea, maybe add it to the js doc of the function?

const today = new Date();

const queryParams = {
country: 'au',
Copy link
Contributor

Choose a reason for hiding this comment

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

Is au country applicable to all sites?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, this is site specific. @alinarublea just showed me how I can integrate the alert config, I will integrate it there

return notFound('Site not found');
}

const auditConfig = site.getAuditConfig();
Copy link
Contributor

Choose a reason for hiding this comment

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

We should run the audit only for sites that are live

}

const ahrefsAPIClient = AhrefsAPIClient.createFrom(context);
const response = await ahrefsAPIClient.getOrganicKeywords(site.getBaseURL(), auditContext);
Copy link
Contributor

Choose a reason for hiding this comment

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

How do you query and get results from ahrefs, using the www or non-www version of the site URL?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

shouldn't it be exactly the same as the saved base url? I tested in ahrefs and if it's not the same it does not return the right results

Copy link
Contributor

Choose a reason for hiding this comment

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

I've checked ahrefs UI with bamboohr.com and it returns no results, whereas www.bamboohr.com does.
Screenshot 2024-01-30 at 14 51 32
Screenshot 2024-01-30 at 14 51 04

Copy link
Contributor Author

Choose a reason for hiding this comment

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

bamboohr.com also forwards to www.bamboohr.com. Shouldn't we have www.bamboohr.com as a baseURL in that case?

sum_traffic: 123,
best_position: 1,
best_position_prev: 1,
best_position_diff: 0,
Copy link
Contributor

Choose a reason for hiding this comment

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

This is something we could easily compute ourselves. How much does it cost to get this field for every keyword every time we run the audit for a site?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's just 1 unit, so I thought it might save us some performance. Is it worth saving one unit?

Copy link
Contributor

Choose a reason for hiding this comment

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

1 unit per row x 15 rows x ~hundred(s) live sites (in later phases) x number of audits scheduled or triggered per month
If we can compute it as easy as this, I'd say we save this 1 unit for fields that we cannot compute ourselves in other ahrefs audit types.

@github-actions
Copy link

This PR will trigger a minor release when merged.

# Conflicts:
#	package-lock.json
#	package.json
@codecov
Copy link

codecov bot commented Jan 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b13fd0f) 100.00% compared to head (efbf2a3) 100.00%.
Report is 20 commits behind head on main.

❗ Current head efbf2a3 differs from pull request most recent head 55bfef9. Consider uploading reports for the commit 55bfef9 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##              main      #112    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files           12        13     +1     
  Lines         1306      1414   +108     
==========================================
+ Hits          1306      1414   +108     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@solaris007 solaris007 added the enhancement New feature or request label Feb 3, 2024
@solaris007
Copy link
Member

no activity, closing

@solaris007 solaris007 closed this Dec 31, 2024
barshat7 pushed a commit to barshat7/spacecat-audit-worker that referenced this pull request Apr 1, 2025
…ion (adobe#112)

rename the markdown handler to text-content as it actually only saves
the text content and no markdown to avoid confusion.
barshat7 pushed a commit to barshat7/spacecat-audit-worker that referenced this pull request Apr 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants