Skip to content

Refactor/dev 141 refactor generic message on notification system#189

Merged
LeonardoVieira1630 merged 6 commits into
refactor/DEV-133_notification_system_slackUIfrom
refactor/DEV-141_refactor_generic_message_on_notification_system
Oct 20, 2025
Merged

Refactor/dev 141 refactor generic message on notification system#189
LeonardoVieira1630 merged 6 commits into
refactor/DEV-133_notification_system_slackUIfrom
refactor/DEV-141_refactor_generic_message_on_notification_system

Conversation

@LeonardoVieira1630

@LeonardoVieira1630 LeonardoVieira1630 commented Oct 16, 2025

Copy link
Copy Markdown
Member

Summary

Improves voting power change notifications by making the direction clear (increased/decreased) and fixing ENS name resolution for wallet addresses.

Changes

1. Clearer Message Direction

  • Before: "⚡ Voting power changed for 0x1f3d...0591 in SCR!"
  • After: "⚡ Voting power increased for gov.blockful.eth in SCR!"

Split generic messages into directional variants:

  • generic.increased - for positive deltas
  • generic.decreased - for negative deltas

2. ENS Resolution Fix

Added address field to metadata for transfer and generic voting power messages, enabling ENS name resolution in consumer services.

Root cause: Metadata wasn't including the wallet address, so the consumer couldn't resolve ENS names for these notification types.

Testing

  • ✅ All 21 voting power tests passing
  • ✅ Build successful
  • ✅ Type checking passed

LeonardoVieira1630 and others added 3 commits October 16, 2025 17:41
Replace generic voting power messages with directional variants:
- Split `generic.changed` into `generic.increased` and `generic.decreased`
- Remove `generic.activity` (was used only for delta=0, now uses increased)

This makes it immediately clear whether voting power went up or down,
improving UX by removing ambiguity from the notification text.

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

Co-Authored-By: Claude <noreply@anthropic.com>
…r messages

Update voting power trigger handler to:
- Use directional messages (increased/decreased) based on delta sign
- Include `address` in metadata for transfer and generic messages
- Enable ENS name resolution in consumer services

This fixes the issue where wallet addresses in transfer and generic
voting power notifications were not being resolved to ENS names.

Before: "⚡ Voting power changed for 0x1f3d...0591 in SCR!"
After: "⚡ Voting power increased for gov.blockful.eth in SCR!"

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

Co-Authored-By: Claude <noreply@anthropic.com>
Update tests to:
- Expect "increased" instead of "changed" for positive/zero deltas
- Expect "decreased" for negative deltas
- Add new test for decreased notification
- Verify metadata includes address field for ENS resolution (transfer & generic)

All 21 tests passing.

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

Co-Authored-By: Claude <noreply@anthropic.com>
@LeonardoVieira1630 LeonardoVieira1630 marked this pull request as ready for review October 16, 2025 20:43
@LeonardoVieira1630 LeonardoVieira1630 self-assigned this Oct 16, 2025
@LeonardoVieira1630 LeonardoVieira1630 added the enhancement New feature or request label Oct 16, 2025

@PedroBinotto PedroBinotto left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

lgtm

LeonardoVieira1630 and others added 3 commits October 20, 2025 19:02
Previously, dispatcher was replacing address placeholders ({{address}},
{{delegator}}, etc.) with raw Ethereum addresses before messages reached
the consumer. This prevented ENS name resolution from occurring.

Changes:
- Remove address parameters from replacePlaceholders() calls in:
  - vote-confirmation-trigger.service.ts
  - voting-power-trigger.service.ts (5 notification types)
  - voting-reminder-trigger.service.ts
- Fix metadata keys to match template placeholders:
  - vote-confirmation: voterAccountId → address
  - voting-reminder: voterAddress → address
- Update unit test assertions to verify placeholders and metadata

Now placeholders remain in messages sent to consumer, which resolves
them to ENS names (e.g., vitalik.eth) using metadata.addresses.

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

Co-Authored-By: Claude <noreply@anthropic.com>
Fixed two issues in ENS name resolution:

1. Placeholder replacement only replaced first occurrence
   - Changed from .replace() to regex with global flag
   - Templates with multiple {{address}} now fully resolve
   - Example: "vitalik.eth voted... vitalik.eth cast vote"
     (instead of "vitalik.eth voted... {{address}} cast vote")

2. Lowercase addresses failed ENS lookup
   - Added getAddress() to convert to EIP-55 checksum format
   - Viem's getEnsName() requires proper checksum addresses
   - Now handles addresses from database (stored as lowercase)

Files changed:
- SlackBotService: Use regex global replacement
- TelegramBotService: Use regex global replacement
- EnsResolverService: Add checksum conversion before lookup

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

Co-Authored-By: Claude <noreply@anthropic.com>
Updated test fixtures to use real Ethereum addresses with ENS names,
enabling visual verification of ENS resolution with SEND_REAL_SLACK=true.

Changes:
- Updated test constants with real addresses:
  - p1: vitalik.eth (0xd8dA6BF26964aF9D7eEd9e03E53415D37aA96045)
  - p2: nick.eth (0x225f137127d9067788314bc7fcc1f36746a3c3B5)
  - p3: brantly.eth (0x983110309620d911731ac0932219af06091b6744)
  - p4: firefish.eth (0xb8c2C29ee19D8307cb7255e1Cd9CbDE883A267d5)

- Updated MockEnsResolverService with real ENS mappings:
  - Added theblackbelt.eth for testing

- Fixed test assertions to expect ENS names instead of raw addresses:
  - Delegation tests: expect "vitalik.eth" not "0xd8dA..."
  - Non-voting tests: expect "firefish.eth" not "nick.eth"
  - Voting power tests: avoid auto-delegation by using different addresses

- Voting reminder tests kept with fake addresses to avoid
  auto-delegation scenarios in test logic

All tests pass with ENS resolution working correctly.

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

Co-Authored-By: Claude <noreply@anthropic.com>
@LeonardoVieira1630 LeonardoVieira1630 merged commit 807ccc1 into refactor/DEV-133_notification_system_slackUI Oct 20, 2025
4 checks passed
@LeonardoVieira1630 LeonardoVieira1630 deleted the refactor/DEV-141_refactor_generic_message_on_notification_system branch October 20, 2025 22:04
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.

3 participants