Feature/whatsapp by y-aithnini · Pull Request #11 · CISCODE-MA/NotificationKit

y-aithnini · 2026-03-11T10:26:50Z

Summary

What does this PR change?

Why

Why is this change needed?

Checklist

Added/updated tests (if behavior changed)
npm run lint passes
npm run typecheck passes
npm test passes
npm run build passes
Added a changeset (npx changeset) if this affects consumers

Notes

Anything reviewers should pay attention to?

* infrastructure adapters * feature/nestjs-integration * merging all features * docs : updated copilot instructions * fix: resolve SonarQube security hotspots and workflow configuration - Replace GitHub Actions version tags with full commit SHA hashes (v6 -> fd88b7d, v1 -> d304d05) - Fix SONAR_PROJECT_KEY from LoggingKit to NotificationKit - Add 'main' branch to PR trigger list for release-check workflow - Resolves 2 security hotspots (githubactions:S7637) for supply chain security * fix: resolve NotificationKit TypeScript type errors - Add explicit NotificationDocument type annotations to map callbacks - Fix mapToRecord signature to accept any type (handles both Map and Record) - Install mongoose as dev dependency for type checking - Fixes pre-push typecheck failures --------- Co-authored-by: yasser <y.aithnini@ciscod.com>

… into feature/comprehensive-testing

Copilot

Pull request overview

This PR introduces concrete repository adapters (Mongoose + in-memory) into the infra layer, updates infra documentation/comments accordingly, and adds a repository-specific Copilot guideline document.

Changes:

Added MongooseNotificationRepository (MongoDB) and InMemoryNotificationRepository implementations of INotificationRepository.
Updated infra exports and documentation to reference the new repositories.
Added .github/copilot-instructions.md and updated package-lock.json.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 11 comments.

Show a summary per file

File	Description
src/infra/repositories/mongoose/notification.schema.ts	Adds a TS suppression comment around optional `mongoose` require.
src/infra/repositories/mongoose/mongoose.repository.ts	New Mongoose-backed repository implementation with indexing and query helpers.
src/infra/repositories/index.ts	Exports the new repository implementations from infra.
src/infra/repositories/in-memory/in-memory.repository.ts	New in-memory repository implementation intended for testing/simple usage.
src/infra/index.ts	Updates infra layer documentation header.
src/infra/README.md	Updates repository usage docs (MongoDB + in-memory).
package-lock.json	Large lockfile update affecting devDependencies and versions.
.github/copilot-instructions.md	Adds extensive contributor/Copilot guidance documentation.

Copilot · 2026-03-11T10:32:51Z

.github/copilot-instructions.md

+  │   │   ├── in-app/                         # In-app adapter
+  │   │   └── webhook/                        # Outbound webhook adapter
+  │   ├── repositories/                       # Persistence adapters
+  │   │   ├── mongodb/                        # Mongoose adapter


The architecture tree references src/infra/repositories/mongodb/, but the codebase currently uses src/infra/repositories/mongoose/. Please align the directory names in this guide with the actual repository structure to avoid confusion.

Suggested change

│ │ ├── mongodb/ # Mongoose adapter

│ │ ├── mongoose/ # Mongoose adapter

Copilot · 2026-03-11T10:32:51Z

.github/copilot-instructions.md

+**Test file location:** same directory as source (`*.spec.ts`)
+
+```
+src/core/
+  ├── notification.service.ts
+  └── notification.service.spec.ts
+
+src/infra/senders/email/
+  ├── email.sender.ts
+  └── email.sender.spec.ts
+```


This guide says test files should be *.spec.ts, but the repository currently uses *.test.ts (e.g., src/core/notification.service.test.ts). Please update the testing section/examples to match the existing test file naming convention.

Copilot · 2026-03-11T10:32:51Z

src/infra/repositories/mongoose/mongoose.repository.ts

+    this.model = this.connection.model<NotificationDocument>(
+      "Notification",
+      schema,
+      this.collectionName,
+    );


The repository accepts a collectionName, but the model is always registered under the fixed name "Notification". With Mongoose’s per-connection model registry, creating another repository instance (same connection) with a different collectionName can either reuse the existing model/collection or throw an overwrite error. Use a model name derived from collectionName (or accept an injected model) to ensure collectionName is actually honored.

Copilot · 2026-03-11T10:32:52Z

src/infra/repositories/mongoose/notification.schema.ts

+    // @ts-ignore - mongoose is an optional peer dependency
    const mongoose = require("mongoose");


Using // @ts-ignore here suppresses all TypeScript errors on the next line and can hide real issues. Prefer a targeted suppression (e.g., eslint disable for no-var-requires) and/or a type assertion like require("mongoose") as typeof import("mongoose") so the line remains type-checked while still supporting the optional peer dependency pattern.

Suggested change

// @ts-ignore - mongoose is an optional peer dependency

const mongoose = require("mongoose");

// eslint-disable-next-line @typescript-eslint/no-var-requires

const mongoose = require("mongoose") as typeof import("mongoose");

Copilot · 2026-03-11T10:32:52Z

src/infra/index.ts

+ * These implementations are internal and not exported by default.
+ * They can be used when configuring the NestJS module.


The comment says infra implementations are "not exported by default", but src/index.ts re-exports ./infra, so they are part of the package’s public surface. Please update this comment to reflect the actual export behavior (or adjust exports if the intent is to keep infra internal).

Suggested change

* These implementations are internal and not exported by default.

* They can be used when configuring the NestJS module.

* These implementations are primarily intended for internal wiring and NestJS module configuration.

* They are re-exported and available to consumers, but are not the preferred abstraction layer.

Copilot · 2026-03-11T10:32:52Z

.github/copilot-instructions.md

+**❌ NEVER export:**
+
+- Internal provider wiring (`createNotificationKitProviders` internals)
+- Raw SDK instances (Nodemailer transporter, Twilio client, Firebase app)
+- Mongoose schema definitions (infrastructure details)


The "NEVER export" list says not to export Mongoose schema definitions, but the package currently exports notificationSchemaDefinition via src/infra/repositories/index.ts. Either update this guidance or stop exporting the schema to keep docs and actual public API consistent.

Suggested change

**❌ NEVER export:**

- Internal provider wiring (`createNotificationKitProviders` internals)

- Raw SDK instances (Nodemailer transporter, Twilio client, Firebase app)

- Mongoose schema definitions (infrastructure details)

**❌ NEVER export (with one documented exception):**

- Internal provider wiring (`createNotificationKitProviders` internals)

- Raw SDK instances (Nodemailer transporter, Twilio client, Firebase app)

- Additional Mongoose schema definitions (infrastructure details) — `notificationSchemaDefinition` is the only intentionally exported schema for advanced integrations

Copilot · 2026-03-11T10:32:53Z

src/infra/repositories/in-memory/in-memory.repository.ts

+    const offset = _criteria.offset || 0;
+    const limit = _criteria.limit || 10;


find() applies a default limit of 10 when _criteria.limit is not provided. This makes the in-memory implementation behave differently from the Mongoose repository (which returns all results unless a limit is specified) and can silently truncate results. Consider only slicing when limit is explicitly set (and treat offset similarly), or default limit to results.length when undefined.

Suggested change

const offset = _criteria.offset || 0;

const limit = _criteria.limit || 10;

const offset = _criteria.offset ?? 0;

const limit = _criteria.limit ?? results.length - offset;

Copilot · 2026-03-11T10:32:53Z

src/infra/repositories/mongoose/mongoose.repository.ts

+      return this.model;
+    }
+
+    const mongoose = (this.connection as any).base;


Avoid casting the Mongoose connection to any to access .base. Connection already exposes base in Mongoose typings; using any hides type errors and makes refactors riskier. Prefer this.connection.base (or destructure Schema from it) with proper typing.

Suggested change

const mongoose = (this.connection as any).base;

const mongoose = this.connection.base;

Copilot · 2026-03-11T10:32:53Z

src/infra/README.md

+import { InMemoryNotificationRepository } from "@ciscode/notification-kit/infra";

-class MyCustomRepository implements INotificationRepository {
-  async create(data: Omit<Notification, "id" | "createdAt" | "updatedAt">): Promise<Notification> {
-    // Your implementation
-  }
+const repository = new InMemoryNotificationRepository();


Same issue as above: @ciscode/notification-kit/infra is not an exported subpath per package.json exports, so this import will not work for consumers. Update to @ciscode/notification-kit or add the appropriate subpath export.

Copilot · 2026-03-11T10:32:53Z

.github/copilot-instructions.md

+### Path Aliases (`tsconfig.json`)
+
+```typescript
+"@/*"       → "src/*"
+"@core/*"   → "src/core/*"
+"@infra/*"  → "src/infra/*"
+"@nest/*"   → "src/nest/*"


This document claims path aliases like @core/*, @infra/*, etc. are configured in tsconfig.json, but the current tsconfig.json has no baseUrl/paths mappings. Either add the actual TS path mappings or remove/update this section to avoid misleading contributors.

Suggested change

### Path Aliases (`tsconfig.json`)

```typescript

"@/*" → "src/*"

"@core/*" → "src/core/*"

"@infra/*" → "src/infra/*"

"@nest/*" → "src/nest/*"

### Optional path aliases for imports

If you want to use the import style shown below, add the following `compilerOptions.paths`

configuration to your `tsconfig.json`:

```jsonc

{

"compilerOptions": {

"baseUrl": "./",

"paths": {

"@/*": ["src/*"],

"@core/*": ["src/core/*"],

"@infra/*": ["src/infra/*"],

"@nest/*": ["src/nest/*"]

}

}

}

- Create whatsapp.utils.ts with shared validation functions - Extract isValidPhoneNumber() to shared utility (removes 20+ lines duplication) - Extract validateWhatsAppRecipient() to shared utility - Extract error messages to WHATSAPP_ERRORS constants - Update TwilioWhatsAppSender to use shared utilities - Update MockWhatsAppSender to use shared utilities - Export utilities from whatsapp index.ts This refactoring reduces code duplication from 24.4% to meet SonarQube Quality Gate requirement (3%).

sonarqubecloud · 2026-03-11T10:37:12Z

Quality Gate failed

Failed conditions
21.7% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

y-aithnini and others added 17 commits February 27, 2026 16:03

implemented decorators and providers

6aadbc9

Add notification and webhook controller tests

ada99be

remove in-memory repository and update exports

571aecc

removed mongoose

73009e2

Merge develop: resolve conflicts and update jest config for spec files

4210d2c

removed duplicate code for sonarqube

96a80f6

docs: add comprehensive documentation for testing implementation

aac7189

style: fix prettier formatting issues

81e390e

Merge branch 'develop' of https://github.com/CISCODE-MA/NotificationKit…

746f00e

… into feature/comprehensive-testing

integrated whatsapp notification msg

2b5c2f4

updated configuration

0e0de92

fix: replace deprecated substr() with slice() in mock WhatsApp sender

804543a

fix: replace Math.random with crypto.randomUUID for secure ID generation

5e2d970

Merge branch 'master' into feature/whatsapp

e2c1e12

fix: change ts-expect-error to ts-ignore in notification.schema.ts

b27dd5d

Merge branch 'develop' into feature/whatsapp

d250e60

Copilot AI review requested due to automatic review settings March 11, 2026 10:26

Copilot started reviewing on behalf of y-aithnini March 11, 2026 10:27 View session

fix: regenerate package-lock.json to sync with package.json

69ae2f6

Copilot AI reviewed Mar 11, 2026

View reviewed changes

y-aithnini closed this Mar 11, 2026

y-aithnini deleted the feature/whatsapp branch March 11, 2026 10:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/whatsapp#11

Feature/whatsapp#11
y-aithnini wants to merge 19 commits intodevelopfrom
feature/whatsapp

y-aithnini commented Mar 11, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

sonarqubecloud bot commented Mar 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	│ │ ├── mongodb/ # Mongoose adapter
	│ │ ├── mongoose/ # Mongoose adapter

		// @ts-ignore - mongoose is an optional peer dependency
		const mongoose = require("mongoose");

		* These implementations are internal and not exported by default.
		* They can be used when configuring the NestJS module.

		const offset = _criteria.offset \|\| 0;
		const limit = _criteria.limit \|\| 10;

	const mongoose = (this.connection as any).base;
	const mongoose = this.connection.base;

-### Path Aliases (`tsconfig.json`)
-```typescript
-"@/*"       → "src/*"
-"@core/*"   → "src/core/*"
-"@infra/*"  → "src/infra/*"
-"@nest/*"   → "src/nest/*"
+### Optional path aliases for imports
+If you want to use the import style shown below, add the following `compilerOptions.paths`
+configuration to your `tsconfig.json`:
+```jsonc
+{
+  "compilerOptions": {
+    "baseUrl": "./",
+    "paths": {
+      "@/*": ["src/*"],
+      "@core/*": ["src/core/*"],
+      "@infra/*": ["src/infra/*"],
+      "@nest/*": ["src/nest/*"]
+    }
+  }
+}

Conversation

y-aithnini commented Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why

Checklist

Notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Mar 11, 2026

Quality Gate failed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

y-aithnini commented Mar 11, 2026 •

edited

Loading