Skip to content

feat(audit-url): add moneyPages source to auditTargetURLs config#1496

Merged
nitinja merged 3 commits intomainfrom
SITES-42547-add-moneyPages-source
Apr 9, 2026
Merged

feat(audit-url): add moneyPages source to auditTargetURLs config#1496
nitinja merged 3 commits intomainfrom
SITES-42547-add-moneyPages-source

Conversation

@nitinja
Copy link
Copy Markdown
Member

@nitinja nitinja commented Apr 1, 2026

Add moneyPages as a new audit target URL source alongside manual, with identical structure (array of { url } entries) and full cross-source deduplication support.

Please ensure your pull request adheres to the following guidelines:

  • make sure to link the related issues in this description
  • when merging / squashing, make sure the fixed issue references are visible in the commits, for easy compilation of release notes

Related Issues

Thanks for contributing!

@nitinja nitinja requested a review from slitviachenko April 1, 2026 21:27
Base automatically changed from SITES-42547-Manage-a-custom-list-of-audit-target-URLs to main April 3, 2026 17:05
manual: Joi.array().items(Joi.object({
url: Joi.string().uri().required(),
})).optional().default([]),
moneyPages: Joi.array().items(Joi.object({
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@nitinja , Let's make sure we can update individual auditTargetURLs keys separately without affecting each other, for example:

% curl -X PATCH \
-d '{
  "config": {
    "auditTargetURLs": {
      "manual": [
        {
          "url": "https://example.com/path1"
        },
        {
          "url": "https://example.com/path2"
        }
      ]
    }
  }
}' \
"https://SPACECAT_API/sites/SITE_ID"

% curl -X PATCH \
-d '{
  "config": {
    "auditTargetURLs": {
      "moneyPages": [
        {
          "url": "https://example.com/money-page1"
        },
        {
          "url": "https://example.com/money-page2"
        }
      ]
    }
  }
}' \
"https://SPACECAT_API/sites/SITE_ID"

In the database, we should have both manual and moneyPages.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

^^^ That way, the API will be more resilient, and we will have less payload input data. Otherwise, to update any individual key, you would need to include all keys to preserve them.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I have taken care of merge here in this PR https://github.com/adobe/spacecat-api-service/pull/2167/changes

Copy link
Copy Markdown
Contributor

@slitviachenko slitviachenko left a comment

Choose a reason for hiding this comment

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

@nitinja , I left a comment, please take a look when you have time

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 9, 2026

This PR will trigger a minor release when merged.

Add moneyPages as a new audit target URL source alongside manual,
with identical structure (array of { url } entries) and full
cross-source deduplication support.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@nitinja nitinja force-pushed the SITES-42547-add-moneyPages-source branch from 2089eb9 to a23f8f0 Compare April 9, 2026 14:20
nitinja and others added 2 commits April 9, 2026 09:39
- Add mergeAuditTargetURLs() method dropped during cherry-pick
- Add TypeScript declaration for mergeAuditTargetURLs
- Strengthen updateAuditTargetURLs test to assert URL values not just length
- Add mergeAuditTargetURLs tests covering per-source independence,
  invalid input guards, and invalid source rejection

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Not called anywhere — updateAuditTargetURLs covers all use cases.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@nitinja nitinja requested a review from slitviachenko April 9, 2026 14:45
@nitinja
Copy link
Copy Markdown
Member Author

nitinja commented Apr 9, 2026

Not that build failure is due to unrelated code, not my changes.

@slitviachenko
Copy link
Copy Markdown
Contributor

@nitinja , Once this PR is deployed, please bump the shared dependency in the PR API.

Copy link
Copy Markdown
Contributor

@slitviachenko slitviachenko left a comment

Choose a reason for hiding this comment

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

LGTM

@nitinja nitinja merged commit 78df8f9 into main Apr 9, 2026
4 of 5 checks passed
@nitinja nitinja deleted the SITES-42547-add-moneyPages-source branch April 9, 2026 16:44
@nitinja
Copy link
Copy Markdown
Member Author

nitinja commented Apr 9, 2026

@nitinja , Once this PR is deployed, please bump the shared dependency in the PR API.

yes sure, waiting for release - thanks

solaris007 pushed a commit that referenced this pull request Apr 10, 2026
## [@adobe/spacecat-shared-data-access-v3.49.0](https://github.com/adobe/spacecat-shared/compare/@adobe/spacecat-shared-data-access-v3.48.0...@adobe/spacecat-shared-data-access-v3.49.0) (2026-04-10)

### Features

* **audit-url:** add moneyPages source to auditTargetURLs config ([#1496](#1496)) ([78df8f9](78df8f9))
@solaris007
Copy link
Copy Markdown
Member

🎉 This PR is included in version @adobe/spacecat-shared-data-access-v3.49.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

nitinja added a commit to adobe/spacecat-api-service that referenced this pull request Apr 10, 2026
Resolves comment
adobe/spacecat-shared#1496 (comment)

Note for validation: now it validates only what the client actually
sent. If the PATCH contains `{ auditTargetURLs: { manual: [...] } }`, we
validate and normalize just manual. The existing moneyPages are carried
over as-is without being re-checked. Only when a client explicitly sends
moneyPages do we validate those.




## Summary

- **Deep-merge `auditTargetURLs` sub-keys on PATCH**
(`src/controllers/sites.js`): the previous shallow merge replaced the
entire `auditTargetURLs` object, so patching only `manual` would wipe
`moneyPages` (and vice versa). Now each source key is merged
independently, allowing callers to update one source without affecting
others.
- **Extend validation to all known sources**
(`src/support/audit-target-urls-validation.js`): validation previously
only covered `manual`. Refactored into a generic per-source helper
(`validateAuditTargetSourceList`) and added `moneyPages` with the same
HTTPS + hostname + max-count + entry-shape checks. Adding future sources
only requires updating `AUDIT_TARGET_URL_SOURCE_LIMITS`.

## Example

Two independent PATCHes now both persist correctly:
```
PATCH /sites/:id  { config: { auditTargetURLs: { manual: [...] } } }
PATCH /sites/:id  { config: { auditTargetURLs: { moneyPages: [...] } } }
```
DB ends up with both `manual` and `moneyPages` intact.

## Test plan

- [x] Existing `auditTargetURLs` unit tests for `manual` pass unchanged
- [x] Add tests: PATCH `manual` only → `moneyPages` preserved in DB
- [x] Add tests: PATCH `moneyPages` only → `manual` preserved in DB
- [x] Add validation tests for `moneyPages` (HTTP rejected, wrong
hostname, over 500 limit, bad entry shape)

🤖 Generated with [Claude Code](https://claude.ai/claude-code)

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants