Skip to content

Fix creative management UI bugs and add audit logging#376

Merged
bokelley merged 3 commits intomainfrom
bokelley/fix-creative-preview-js
Oct 13, 2025
Merged

Fix creative management UI bugs and add audit logging#376
bokelley merged 3 commits intomainfrom
bokelley/fix-creative-preview-js

Conversation

@bokelley
Copy link
Collaborator

Summary

Fixes critical bugs in creative management UI and adds comprehensive audit logging for compliance.

Issues Fixed

1. JavaScript Syntax Error - View Preview Button ✅

Error: SyntaxError: Unexpected end of script

Root Cause: Inlining JSON data in onclick handler broke when creative data contained quotes/special characters.

Fix:

  • Store creative data in data-creative-data attribute with proper escaping
  • Parse data inside function instead of passing as parameter
  • Added error handling for JSON parsing

2. 400 Bad Request - Approve Creative ✅

Error: 400 Bad Request: The browser (or proxy) sent a request that this server could not understand

Root Cause: POST request had no body; backend expects approved_by field.

Fix:

  • Added request body with required fields
  • Proper error handling with response.ok check
  • Console logging for debugging

3. Missing Fields - Reject Creative ✅

Root Cause: Backend expects rejection_reason and rejected_by, not reason.

Fix:

  • Use correct field names per backend API
  • Validate rejection reason is not empty
  • Update UI prompt to indicate reason is required

Enhancement: Complete Audit Trail ✅

Added audit logging to both approve and reject operations:

  • Logs to audit_logs table via AuditLogger.log_operation()
  • Tracks: operation, reviewer, creative details, human override flag
  • Appears automatically in activity stream

Complete Notification Chain

Both operations now fire:

  1. ✅ Database tracking (CreativeReview records)
  2. ✅ Slack notifications (if configured)
  3. ✅ Webhook calls (if configured)
  4. ✅ Audit logging (compliance trail)

Testing

  • ✅ All unit tests pass (593 passed)
  • ✅ All integration tests pass (192 passed)
  • ✅ Pre-commit hooks pass
  • Manual testing confirmed all three buttons work correctly

Files Changed

  • templates/creative_management.html - Fixed JS errors and API calls
  • src/admin/blueprints/creatives.py - Added audit logging

Note

Pre-existing mypy errors in audit_logger.py and other modules are not related to these changes. My usage of log_operation() is correct and type-safe per the function signature.

- Changed onclick handler to not inline JSON data (was causing syntax errors)
- Store creative data in data-creative-data attribute with proper escaping
- Parse data inside viewCreativeDetails function instead
- Removed unnecessary async keyword from viewCreativeDetails

Fixes: [Error] SyntaxError: Unexpected end of script at review:221
- Added missing request body to approve endpoint (approved_by)
- Fixed reject endpoint to use correct field names (rejection_reason, rejected_by)
- Added validation to require rejection reason
- Improved error handling with response.ok checks
- Added console.error logging for debugging

Fixes: 400 Bad Request errors when approving/rejecting creatives
- Import AuditLogger in both approve_creative and reject_creative functions
- Log operations to database via log_operation() method
- Track: operation name, reviewer, creative details, human override flag
- Audit logs appear in activity stream for compliance tracking
- Completes the notification/reporting chain: Slack + Webhooks + Audit Trail

Note: Pre-existing mypy errors in audit_logger.py, slack_notifier.py, and
other modules are not related to these changes. My usage of log_operation()
is correct and type-safe.

Related to security and compliance requirements
@bokelley bokelley merged commit 5072acd into main Oct 13, 2025
8 checks passed
bokelley added a commit that referenced this pull request Oct 14, 2025
Merged latest changes from main including:
- Product pricing display and migration to pricing_options table (#369)
- Update media buy fix for database-persisted buys (#380)
- Automatic creative preview fetching (#379)
- Creative sync improvements (#378, #376, #373)
- Budget field access fixes (#374)
- UI improvements (#377, #375, #372)

Conflict Resolution:
- src/core/main.py: Combined debug logging from main with helper function from our branch
- Kept both improvements: stderr debug prints + create_get_products_request() helper

No conflicts in schema files - automatic merges succeeded.
danf-newton pushed a commit to Newton-Research-Inc/salesagent that referenced this pull request Nov 24, 2025
* Fix JavaScript syntax error in creative preview button

- Changed onclick handler to not inline JSON data (was causing syntax errors)
- Store creative data in data-creative-data attribute with proper escaping
- Parse data inside viewCreativeDetails function instead
- Removed unnecessary async keyword from viewCreativeDetails

Fixes: [Error] SyntaxError: Unexpected end of script at review:221

* Fix approve/reject creative API calls

- Added missing request body to approve endpoint (approved_by)
- Fixed reject endpoint to use correct field names (rejection_reason, rejected_by)
- Added validation to require rejection reason
- Improved error handling with response.ok checks
- Added console.error logging for debugging

Fixes: 400 Bad Request errors when approving/rejecting creatives

* Add audit logging to creative approve/reject operations

- Import AuditLogger in both approve_creative and reject_creative functions
- Log operations to database via log_operation() method
- Track: operation name, reviewer, creative details, human override flag
- Audit logs appear in activity stream for compliance tracking
- Completes the notification/reporting chain: Slack + Webhooks + Audit Trail

Note: Pre-existing mypy errors in audit_logger.py, slack_notifier.py, and
other modules are not related to these changes. My usage of log_operation()
is correct and type-safe.

Related to security and compliance requirements
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant