Skip to content

feat(gmail): add history types filter to watch serve#168

Open
salmonumbrella wants to merge 5 commits intosteipete:mainfrom
salmonumbrella:fix/issue-165-history-types
Open

feat(gmail): add history types filter to watch serve#168
salmonumbrella wants to merge 5 commits intosteipete:mainfrom
salmonumbrella:fix/issue-165-history-types

Conversation

@salmonumbrella
Copy link
Contributor

Summary

  • Add --history-types flag to gmail watch serve command to filter which Gmail history event types are fetched
  • Default to messageAdded for backward compatibility (previously hardcoded)
  • Separate deleted message IDs from fetchable messages to avoid 404 errors when fetching deleted messages
  • Include deletedMessageIds field in webhook payload for consumers that need to track deletions

Code Review Fixes

  • Add canonical forms to history type alias map for robustness
  • Improve test assertion precision with exact count verification
  • Extract duplicate state update logic into updateStateAfterHistory helper
  • Add tests for empty input edge cases
  • Update documentation to clarify default behavior

Fixes #165

Test Plan

  • All existing tests pass
  • New tests for default history types behavior
  • New tests for deleted message ID separation
  • New tests for empty input edge cases
  • Integration test for history types filtering

🤖 Generated with Claude Code

salmonumbrella and others added 5 commits February 2, 2026 20:09
The commit 5137fcb added --history-types flag but changed the default
behavior: when not specified, it now fetched ALL history types instead
of just messageAdded (the previous hardcoded behavior).

This fix ensures backward compatibility by defaulting to messageAdded
when --history-types is not provided.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When messageDeleted history type is requested, deleted messages cannot
be fetched from Gmail API (returns 404). Now collectHistoryMessageIDs
returns a struct with separate FetchIDs and DeletedIDs lists, and the
hook payload includes deletedMessageIds field for consumers.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add canonical forms to history type alias map for robustness
- Add length assertion for FetchIDs in TestCollectHistoryMessageIDs
- Clarify documentation that default is messageAdded
- Add test for empty input edge case in parseHistoryTypes
- Extract duplicate state update logic into updateStateAfterHistory helper

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Rename inner `err` to `updateErr` to avoid shadowing the outer
variable, satisfying golangci-lint's govet shadow checker.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Copy link

@niemesrw niemesrw left a comment

Choose a reason for hiding this comment

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

PR Review: feat(gmail): add history types filter to watch serve

Summary

This PR adds a --history-types flag to gmail watch serve to filter which Gmail history event types are fetched. It addresses issue #165.

What it does

  1. Adds --history-types flag accepting messageAdded, messageDeleted, labelAdded, labelRemoved
  2. Defaults to messageAdded for backward compatibility (previously hardcoded)
  3. Separates deleted message IDs from fetchable messages to avoid 404 errors
  4. Adds deletedMessageIds field in webhook payload for consumers tracking deletions

Code Quality

Positives:

  • Good backward compatibility by defaulting to messageAdded
  • Smart alias handling for history types (case-insensitive, plural forms like messagesAdded)
  • Clean extraction of updateStateAfterHistory helper to reduce duplication
  • Comprehensive test coverage including edge cases (empty input, add-then-delete ordering)
  • Documentation updated in docs/watch.md

Minor observations:

  1. gmail_watch_server.go:177-180 - The condition if len(s.cfg.HistoryTypes) > 0 is defensive but effectively always true since parseHistoryTypes() returns ["messageAdded"] by default. Not a bug, just slightly redundant.

  2. The collectHistoryMessageIDs function uses linear scan for removal (O(n) per deletion) which is fine for typical payload sizes.

  3. The handling of add-then-delete ordering is correct - if a message is added then later deleted in the same history response, it correctly ends up only in DeletedIDs.

Test Coverage

  • TestCollectHistoryMessageIDs - verifies fetch/deleted separation
  • TestCollectHistoryMessageIDs_DeletedRemovesFromFetch - verifies ordering
  • TestCollectHistoryMessageIDs_EmptyResponse - nil/empty edge cases
  • TestParseHistoryTypes* - parsing, defaults, and empty string handling
  • TestGmailWatchServer_ServeHTTP_HistoryTypes_NoMatch - integration test

Verdict

This is a well-implemented feature. The code is clean, follows existing patterns, has good test coverage, and maintains backward compatibility. Ready to merge.

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.

gmail watch serve: Add --history-types flag to filter notification types

2 participants