Skip to content

ForwardEmail.net support#10597

Merged
nbudin merged 6 commits into
mainfrom
forwardemail
Apr 20, 2025
Merged

ForwardEmail.net support#10597
nbudin merged 6 commits into
mainfrom
forwardemail

Conversation

@nbudin
Copy link
Copy Markdown
Contributor

@nbudin nbudin commented Apr 20, 2025

No description provided.

@nbudin nbudin added enhancement minor Bumps the minor version number on release labels Apr 20, 2025
@nbudin nbudin requested a review from Copilot April 20, 2025 03:22
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for ForwardEmail.net by introducing a new service and job to handle email forwarding, as well as integrating email forwarding sync callbacks into various models. Key changes include:

  • Adding the SyncForwardEmailService to handle API interactions with ForwardEmail.net.
  • Implementing email forwarding sync callbacks in TeamMember, StaffPosition, Event, EmailRoute, and Convention models.
  • Enhancing EmailForwardingRouter with Mapping and MappingSet to consolidate email route mappings.

Reviewed Changes

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

Show a summary per file
File Description
app/services/sync_forward_email_service.rb New service to connect and synchronize domains and aliases with ForwardEmail.net, with a potential loop condition issue in pagination.
app/services/email_forwarding_router.rb New Mapping and MappingSet classes to assemble email forwarding mappings; potential missing implementation for keys method.
app/models/team_member.rb Added after_commit callback to trigger email forwarding sync.
app/models/staff_position.rb Added after_commit callback; potential issue with referencing an undefined association.
app/models/event.rb Added after_commit callback for email forwarding sync.
app/models/email_route.rb Added after_commit callback for email forwarding sync with improved normalization methods.
app/models/convention.rb Updated callback to conditionally trigger email forwarding sync on multiple domain/setting changes.
app/jobs/sync_email_forwarding_for_domain_job.rb New job to trigger synchronization using the service.
Gemfile Added Faraday gem to support HTTP connections.

Comment thread app/services/sync_forward_email_service.rb Outdated
.merge(all_email_route_mappings)
end

def self.all_inbound_addresses
Copy link

Copilot AI Apr 20, 2025

Choose a reason for hiding this comment

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

MappingSet instances do not implement a 'keys' method; consider delegating 'keys' to the underlying mappings hash or adjusting the method accordingly.

Copilot uses AI. Check for mistakes.
end

def sync_email_forwarding
SyncEmailForwardingForDomainJob.perform_later(EmailRoute.normalize_domain(convention.domain))
Copy link

Copilot AI Apr 20, 2025

Choose a reason for hiding this comment

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

The method references 'convention', which is not defined on StaffPosition; consider using the associated 'catch_all_convention' if that was the intended source.

Suggested change
SyncEmailForwardingForDomainJob.perform_later(EmailRoute.normalize_domain(convention.domain))
domain = convention&.domain || catch_all_convention&.domain
return unless domain
SyncEmailForwardingForDomainJob.perform_later(EmailRoute.normalize_domain(domain))

Copilot uses AI. Check for mistakes.
nbudin and others added 2 commits April 19, 2025 20:23
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown
Contributor

Code Coverage Report: Only Changed Files listed

Package Coverage
Overall Coverage 🟢 30.61%

Minimum allowed coverage is 0%, this run produced 30.61%

@nbudin nbudin merged commit 9727d21 into main Apr 20, 2025
17 checks passed
@nbudin nbudin deleted the forwardemail branch April 20, 2025 04:37
@sentry
Copy link
Copy Markdown

sentry Bot commented Apr 20, 2025

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ Faraday::TooManyRequestsError: the server responded with status 429 (Faraday::TooManyRequestsError) SyncForwardEmailService in add_alias View Issue
  • ‼️ Faraday::BadRequestError: the server responded with status 400 (Faraday::BadRequestError) SyncForwardEmailService in add_alias View Issue
  • ‼️ Faraday::BadRequestError: the server responded with status 400 (Faraday::BadRequestError) SyncEmailForwardingForDomainJob View Issue

Did you find this useful? React with a 👍 or 👎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement minor Bumps the minor version number on release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants