Skip to content

feat(shared): add audit package - #172

Open
ginaxu1 wants to merge 1 commit into
mainfrom
feat/shared-audit
Open

feat(shared): add audit package#172
ginaxu1 wants to merge 1 commit into
mainfrom
feat/shared-audit

Conversation

@ginaxu1

@ginaxu1 ginaxu1 commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Land the shared/audit package on its own so it can be tagged as a real shared release (e.g. shared/v0.4.0) before payment and storage start importing it.

Auditor is about to become part of the exported payment and storage service interfaces. Those modules cannot require github.com/OpenNSW/core/shared v0.3.0 for a package that does not exist at that tag, and a replace => ../shared directive is not transitive — it only works when the declaring module is built from this monorepo checkout.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • Other (please describe):

Changes Made

  • Add shared/audit with Auditor, Event, Domain, and Action constants used by payment and storage.
  • Document the package in shared/README.md.

No consumer (payment / storage) changes in this PR.

Testing

  • I have tested this change locally
  • I have added tests that prove my fix is effective or that my feature works
  • I have tested edge cases
  • All existing tests pass

GOWORK=off go test -C shared ./... passes. The new package is types-only (no test files).

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have checked that there are no merge conflicts

Related Issues

Split out of #149 at reviewer request. After this merges, please tag shared/v0.4.0 so #149 can bump require github.com/OpenNSW/core/shared in payment and storage and drop the local replace directives.

@coderabbitai

coderabbitai Bot commented Aug 27, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: a1a6730a-1e68-4d35-81f8-0d23fc0f319f


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@ginaxu1 ginaxu1 mentioned this pull request Aug 27, 2026
17 tasks
@mushrafmim

Copy link
Copy Markdown
Contributor

Event currently grows a set of fields per domain (GatewayID/Reference/Status for payment, Key/Filename/MimeType/Size for storage), with the comment "switch on Domain first" as the only thing keeping domains from reading each other's zero-valued fields. As more domains onboard (auth, notifications, ...), this struct grows without bound and loses any compile-time guarantee that a payment event doesn't carry storage fields.

Suggest replacing the domain-specific fields with a Details interface, so each domain owns its own payload type instead of shared/audit growing a field per domain:

type Details interface {
    Metadata() map[string]any
}

type Event struct {
    TraceID    string
    Timestamp  time.Time
    EventType  string
    Action     Action
    Status     Status
    ActorType  string
    ActorID    string
    TargetType string
    TargetID   string

    Details Details // nil when a domain has nothing extra to attach
}

payment.AuditDetails / storage.AuditDetails would live in their own packages and implement Metadata().

Worth doing now, while there are no consumers of this package yet. It also happens to line up field-for-field with github.com/LSFLK/argus/pkg/audit.AuditLogRequest (Action already uses Argus's CREATE/READ/UPDATE/DELETE values, Status SUCCESS/FAILURE instead of a Failure bool) — so if this is meant to eventually feed Argus, a bridge to AuditLogRequest becomes a straight field copy rather than a translation layer.

Land Auditor, Event, and domain/action constants in shared/audit so
downstream modules can depend on a tagged shared release.

Co-authored-by: Cursor <cursoragent@cursor.com>
@ginaxu1

ginaxu1 commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Done — Event no longer has per-domain fields.

shared/audit now has Argus-aligned Action/Status plus a Details interface (Metadata() map[string]any). Domain payloads stay in the emitting packages (payment.AuditDetails / storage.AuditDetails on #149).

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.

2 participants