feat: add account toggle, message dedup tracking, SMTP per-user and Gmail OAuth flow plan
Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com> Agent-Logs-Url: https://github.com/christianlouis/pop_puller_to_gmail/sessions/b77800ca-b452-4427-b0bf-63403e906916
This commit is contained in:
@@ -8,7 +8,7 @@ from sqlalchemy import select, desc
|
||||
from app.core.database import get_db
|
||||
from app.core.deps import get_current_active_user
|
||||
from app.core.security import encrypt_credential
|
||||
from app.models.database_models import User, MailAccount
|
||||
from app.models.database_models import User, MailAccount, AccountStatus
|
||||
from app.models.schemas import (
|
||||
MailAccountCreate,
|
||||
MailAccountResponse,
|
||||
@@ -182,6 +182,38 @@ async def delete_mail_account(
|
||||
await db.commit()
|
||||
|
||||
|
||||
@router.patch("/{account_id}/toggle", response_model=MailAccountResponse)
|
||||
async def toggle_mail_account(
|
||||
account_id: int,
|
||||
current_user: User = Depends(get_current_active_user),
|
||||
db: AsyncSession = Depends(get_db),
|
||||
):
|
||||
"""Toggle the enabled/disabled state of a mail account"""
|
||||
result = await db.execute(
|
||||
select(MailAccount).where(
|
||||
MailAccount.id == account_id, MailAccount.user_id == current_user.id
|
||||
)
|
||||
)
|
||||
account = result.scalar_one_or_none()
|
||||
|
||||
if not account:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_404_NOT_FOUND, detail="Mail account not found"
|
||||
)
|
||||
|
||||
account.is_enabled = not account.is_enabled # type: ignore[assignment]
|
||||
|
||||
# When re-enabling a previously errored account, reset status to ACTIVE
|
||||
# so the scheduler picks it up on the next run.
|
||||
if account.is_enabled and account.status == AccountStatus.ERROR:
|
||||
account.status = AccountStatus.ACTIVE # type: ignore[assignment]
|
||||
|
||||
await db.commit()
|
||||
await db.refresh(account)
|
||||
|
||||
return account
|
||||
|
||||
|
||||
@router.post("/test", response_model=MailAccountTestResponse)
|
||||
async def test_mail_connection(
|
||||
test_request: MailAccountTestRequest,
|
||||
|
||||
@@ -446,6 +446,44 @@ class AuditLog(Base):
|
||||
)
|
||||
|
||||
|
||||
class DownloadedMessageId(Base):
|
||||
"""
|
||||
Tracks unique message IDs that have already been downloaded and forwarded.
|
||||
|
||||
- For POP3: stores the UIDL string returned by the server.
|
||||
- For IMAP: stores the IMAP UID (numeric string) of the message.
|
||||
|
||||
This prevents re-processing the same message when delete_after_forward=False.
|
||||
"""
|
||||
|
||||
__tablename__ = "downloaded_message_ids"
|
||||
|
||||
id = Column(Integer, primary_key=True, index=True)
|
||||
mail_account_id = Column(
|
||||
Integer, ForeignKey("mail_accounts.id", ondelete="CASCADE"), nullable=False
|
||||
)
|
||||
|
||||
# Unique message identifier (UIDL for POP3, UID for IMAP)
|
||||
message_uid = Column(String(512), nullable=False)
|
||||
|
||||
downloaded_at = Column(
|
||||
DateTime(timezone=True),
|
||||
default=lambda: datetime.now(timezone.utc),
|
||||
nullable=False,
|
||||
)
|
||||
|
||||
# Indexes — unique constraint prevents duplicates
|
||||
__table_args__ = (
|
||||
Index(
|
||||
"idx_account_message_uid",
|
||||
"mail_account_id",
|
||||
"message_uid",
|
||||
unique=True,
|
||||
),
|
||||
Index("idx_downloaded_at", "downloaded_at"),
|
||||
)
|
||||
|
||||
|
||||
class GmailCredential(Base):
|
||||
"""Stores OAuth2 credentials for Gmail API access (per-user)"""
|
||||
|
||||
|
||||
@@ -11,7 +11,7 @@ from email import parser
|
||||
from email.mime.text import MIMEText
|
||||
from email.mime.multipart import MIMEMultipart
|
||||
from email.utils import formatdate, make_msgid
|
||||
from typing import List, Dict, Any, Optional, Tuple
|
||||
from typing import List, Dict, Any, Optional, Set, Tuple
|
||||
import logging
|
||||
from aioimaplib import aioimaplib
|
||||
|
||||
@@ -143,26 +143,42 @@ class MailProcessor:
|
||||
except Exception as e:
|
||||
return False, f"IMAP connection failed: {str(e)}"
|
||||
|
||||
async def fetch_emails(self, max_count: Optional[int] = None) -> List[bytes]:
|
||||
async def fetch_emails(
|
||||
self,
|
||||
max_count: Optional[int] = None,
|
||||
already_seen_uids: Optional[Set[str]] = None,
|
||||
) -> Tuple[List[bytes], List[str]]:
|
||||
"""
|
||||
Fetch emails from the mail server.
|
||||
Returns list of raw email data.
|
||||
|
||||
Args:
|
||||
max_count: Maximum number of messages to fetch.
|
||||
already_seen_uids: Set of message UIDs that have already been
|
||||
processed and should be skipped.
|
||||
|
||||
Returns:
|
||||
A tuple of (raw_email_bytes_list, new_uid_strings_list).
|
||||
The caller should persist the new UIDs to prevent re-processing.
|
||||
"""
|
||||
effective_max: int = max_count if max_count is not None else self.account.max_emails_per_check # type: ignore[assignment]
|
||||
seen: Set[str] = already_seen_uids or set()
|
||||
|
||||
if self.account.protocol in [MailProtocol.POP3, MailProtocol.POP3_SSL]:
|
||||
return await self._fetch_pop3_emails(effective_max)
|
||||
return await self._fetch_pop3_emails(effective_max, seen)
|
||||
else:
|
||||
return await self._fetch_imap_emails(effective_max)
|
||||
return await self._fetch_imap_emails(effective_max, seen)
|
||||
|
||||
async def _fetch_pop3_emails(self, max_count: int) -> List[bytes]:
|
||||
"""Fetch emails via POP3"""
|
||||
emails = []
|
||||
async def _fetch_pop3_emails(
|
||||
self, max_count: int, already_seen_uids: Set[str]
|
||||
) -> Tuple[List[bytes], List[str]]:
|
||||
"""Fetch emails via POP3, skipping already-downloaded UIDs."""
|
||||
emails: List[bytes] = []
|
||||
new_uids: List[str] = []
|
||||
|
||||
try:
|
||||
loop = asyncio.get_event_loop()
|
||||
|
||||
def fetch_pop3():
|
||||
def fetch_pop3() -> Tuple[List[bytes], List[str]]:
|
||||
# Connect
|
||||
if self.account.protocol == MailProtocol.POP3_SSL:
|
||||
context = ssl.create_default_context()
|
||||
@@ -181,27 +197,49 @@ class MailProcessor:
|
||||
pop_conn.user(self.account.username)
|
||||
pop_conn.pass_(self.password)
|
||||
|
||||
# Get message count
|
||||
num_messages = len(pop_conn.list()[1])
|
||||
# Retrieve UIDL map: {msg_number: uid_string}
|
||||
uidl_response = pop_conn.uidl()
|
||||
uid_map: Dict[int, str] = {}
|
||||
for entry in uidl_response[1]:
|
||||
parts = entry.decode().split(" ", 1)
|
||||
if len(parts) == 2:
|
||||
uid_map[int(parts[0])] = parts[1].strip()
|
||||
|
||||
num_messages = len(uid_map)
|
||||
logger.info(
|
||||
f"Found {num_messages} messages for account {self.account.id}"
|
||||
)
|
||||
|
||||
fetched_emails = []
|
||||
messages_to_delete = []
|
||||
fetched: List[bytes] = []
|
||||
fetched_uids: List[str] = []
|
||||
messages_to_delete: List[int] = []
|
||||
fetched_count = 0
|
||||
|
||||
for msg_num, uid in uid_map.items():
|
||||
if fetched_count >= max_count:
|
||||
break
|
||||
|
||||
# Skip messages we already processed
|
||||
if uid in already_seen_uids:
|
||||
logger.debug(
|
||||
f"Skipping already-downloaded message {uid} "
|
||||
f"for account {self.account.id}"
|
||||
)
|
||||
continue
|
||||
|
||||
# Fetch emails (limited by max_count)
|
||||
for i in range(1, min(num_messages + 1, max_count + 1)):
|
||||
try:
|
||||
response, lines, octets = pop_conn.retr(i)
|
||||
response, lines, octets = pop_conn.retr(msg_num)
|
||||
email_data = b"\r\n".join(lines)
|
||||
fetched_emails.append(email_data)
|
||||
messages_to_delete.append(i)
|
||||
fetched.append(email_data)
|
||||
fetched_uids.append(uid)
|
||||
messages_to_delete.append(msg_num)
|
||||
fetched_count += 1
|
||||
logger.info(
|
||||
f"Retrieved message {i} from account {self.account.id}"
|
||||
f"Retrieved message {msg_num} (uid={uid}) "
|
||||
f"from account {self.account.id}"
|
||||
)
|
||||
except Exception as e:
|
||||
logger.error(f"Error retrieving message {i}: {e}")
|
||||
logger.error(f"Error retrieving message {msg_num}: {e}")
|
||||
|
||||
# Delete messages if configured
|
||||
if self.account.delete_after_forward:
|
||||
@@ -212,19 +250,22 @@ class MailProcessor:
|
||||
logger.error(f"Error deleting message {msg_id}: {e}")
|
||||
|
||||
pop_conn.quit()
|
||||
return fetched_emails
|
||||
return fetched, fetched_uids
|
||||
|
||||
emails = await loop.run_in_executor(None, fetch_pop3)
|
||||
emails, new_uids = await loop.run_in_executor(None, fetch_pop3)
|
||||
|
||||
except Exception as e:
|
||||
logger.error(f"Error fetching POP3 emails: {e}")
|
||||
raise MailFetchError(f"POP3 fetch error: {str(e)}")
|
||||
|
||||
return emails
|
||||
return emails, new_uids
|
||||
|
||||
async def _fetch_imap_emails(self, max_count: int) -> List[bytes]:
|
||||
"""Fetch emails via IMAP"""
|
||||
emails = []
|
||||
async def _fetch_imap_emails(
|
||||
self, max_count: int, already_seen_uids: Set[str]
|
||||
) -> Tuple[List[bytes], List[str]]:
|
||||
"""Fetch emails via IMAP, marking each message \Seen to prevent re-fetch."""
|
||||
emails: List[bytes] = []
|
||||
new_uids: List[str] = []
|
||||
|
||||
try:
|
||||
# Create IMAP client
|
||||
@@ -241,8 +282,8 @@ class MailProcessor:
|
||||
await imap_client.login(self.account.username, self.password)
|
||||
await imap_client.select("INBOX")
|
||||
|
||||
# Search for all messages
|
||||
response = await imap_client.search("UNSEEN") # Only fetch unread
|
||||
# Search for unseen messages only
|
||||
response = await imap_client.search("UNSEEN")
|
||||
message_ids = response.lines[0].split()
|
||||
|
||||
# Limit to max_count
|
||||
@@ -254,6 +295,18 @@ class MailProcessor:
|
||||
|
||||
# Fetch each message
|
||||
for msg_id in message_ids:
|
||||
uid_str = msg_id.decode() if isinstance(msg_id, bytes) else str(msg_id)
|
||||
|
||||
# Skip messages already tracked in our DB
|
||||
if uid_str in already_seen_uids:
|
||||
logger.debug(
|
||||
f"Skipping already-processed IMAP message {uid_str} "
|
||||
f"for account {self.account.id}"
|
||||
)
|
||||
# Still mark as Seen so it doesn't show up in UNSEEN searches
|
||||
await imap_client.store(msg_id, "+FLAGS", "\\Seen")
|
||||
continue
|
||||
|
||||
try:
|
||||
response = await imap_client.fetch(msg_id, "(RFC822)")
|
||||
|
||||
@@ -272,8 +325,12 @@ class MailProcessor:
|
||||
|
||||
if email_data:
|
||||
emails.append(email_data)
|
||||
new_uids.append(uid_str)
|
||||
|
||||
# Always mark as Seen after fetching so the message is
|
||||
# not picked up again on the next UNSEEN search.
|
||||
await imap_client.store(msg_id, "+FLAGS", "\\Seen")
|
||||
|
||||
# Mark as seen if deleting after forward
|
||||
if self.account.delete_after_forward:
|
||||
await imap_client.store(msg_id, "+FLAGS", "\\Deleted")
|
||||
|
||||
@@ -290,7 +347,7 @@ class MailProcessor:
|
||||
logger.error(f"Error fetching IMAP emails: {e}")
|
||||
raise MailFetchError(f"IMAP fetch error: {str(e)}")
|
||||
|
||||
return emails
|
||||
return emails, new_uids
|
||||
|
||||
@staticmethod
|
||||
async def forward_email(
|
||||
|
||||
@@ -17,12 +17,13 @@ from app.models.database_models import (
|
||||
AccountStatus,
|
||||
DeliveryMethod,
|
||||
GmailCredential,
|
||||
DownloadedMessageId,
|
||||
)
|
||||
from app.services.mail_processor import MailProcessor
|
||||
from app.services.gmail_service import GmailService
|
||||
from app.services.config_service import ConfigService
|
||||
from app.core.config import settings
|
||||
from sqlalchemy import select, and_
|
||||
from sqlalchemy import select, and_, delete
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@@ -69,11 +70,22 @@ async def process_mail_account(account_id: int):
|
||||
# Decrypt password
|
||||
password = decrypt_credential(account.encrypted_password) # type: ignore[arg-type]
|
||||
|
||||
# Load already-downloaded UIDs to prevent re-processing
|
||||
seen_result = await db.execute(
|
||||
select(DownloadedMessageId.message_uid).where(
|
||||
DownloadedMessageId.mail_account_id == account.id
|
||||
)
|
||||
)
|
||||
already_seen_uids = set(seen_result.scalars().all())
|
||||
|
||||
# Create processor
|
||||
processor = MailProcessor(account, password)
|
||||
|
||||
# Fetch emails
|
||||
emails = await processor.fetch_emails(account.max_emails_per_check) # type: ignore[arg-type]
|
||||
# Fetch emails (returns raw bytes + new UIDs)
|
||||
emails, new_uids = await processor.fetch_emails(
|
||||
account.max_emails_per_check, # type: ignore[arg-type]
|
||||
already_seen_uids=already_seen_uids,
|
||||
)
|
||||
|
||||
run.emails_fetched = len(emails) # type: ignore[assignment]
|
||||
|
||||
@@ -130,7 +142,9 @@ async def process_mail_account(account_id: int):
|
||||
await db.commit()
|
||||
return
|
||||
|
||||
for email_data in emails:
|
||||
successfully_forwarded_uids: list[str] = []
|
||||
|
||||
for email_data, uid in zip(emails, new_uids):
|
||||
try:
|
||||
if use_gmail_api and gmail_service:
|
||||
# Inject via Gmail API (preferred)
|
||||
@@ -140,6 +154,7 @@ async def process_mail_account(account_id: int):
|
||||
source_account_name=account.name, # type: ignore[arg-type]
|
||||
)
|
||||
emails_forwarded += 1
|
||||
successfully_forwarded_uids.append(uid)
|
||||
else:
|
||||
# Forward via SMTP (fallback)
|
||||
success = await MailProcessor.forward_email(
|
||||
@@ -147,6 +162,7 @@ async def process_mail_account(account_id: int):
|
||||
)
|
||||
if success:
|
||||
emails_forwarded += 1
|
||||
successfully_forwarded_uids.append(uid)
|
||||
else:
|
||||
emails_failed += 1
|
||||
|
||||
@@ -154,6 +170,16 @@ async def process_mail_account(account_id: int):
|
||||
logger.error(f"Error delivering email: {e}")
|
||||
emails_failed += 1
|
||||
|
||||
# Persist new message UIDs so they are not processed again
|
||||
for uid in successfully_forwarded_uids:
|
||||
if uid not in already_seen_uids:
|
||||
db.add(
|
||||
DownloadedMessageId(
|
||||
mail_account_id=account.id,
|
||||
message_uid=uid,
|
||||
)
|
||||
)
|
||||
|
||||
# Update run
|
||||
run.emails_forwarded = emails_forwarded # type: ignore[assignment]
|
||||
run.emails_failed = emails_failed # type: ignore[assignment]
|
||||
@@ -210,15 +236,11 @@ async def process_all_enabled_accounts():
|
||||
"""
|
||||
async with async_session_maker() as db:
|
||||
try:
|
||||
# Get all enabled accounts
|
||||
# Fetch all enabled accounts regardless of operational status so
|
||||
# that accounts in ERROR state are retried automatically.
|
||||
result = await db.execute(
|
||||
select(MailAccount).where(
|
||||
and_(
|
||||
MailAccount.is_enabled == True, # noqa: E712
|
||||
MailAccount.status.in_(
|
||||
[AccountStatus.ACTIVE, AccountStatus.TESTING]
|
||||
),
|
||||
)
|
||||
MailAccount.is_enabled == True, # noqa: E712
|
||||
)
|
||||
)
|
||||
accounts = result.scalars().all()
|
||||
@@ -248,10 +270,10 @@ async def process_all_enabled_accounts():
|
||||
@celery_app.task(base=AsyncTask, name="app.workers.tasks.cleanup_old_logs")
|
||||
async def cleanup_old_logs(days_to_keep: int = 30):
|
||||
"""
|
||||
Clean up old processing logs and runs.
|
||||
Clean up old processing logs, runs, and downloaded message ID records.
|
||||
|
||||
Args:
|
||||
days_to_keep: Number of days of logs to retain
|
||||
days_to_keep: Number of days of data to retain
|
||||
"""
|
||||
async with async_session_maker() as db:
|
||||
try:
|
||||
@@ -275,6 +297,14 @@ async def cleanup_old_logs(days_to_keep: int = 30):
|
||||
for log in old_logs:
|
||||
await db.delete(log)
|
||||
|
||||
# Delete old downloaded message ID records so the table doesn't
|
||||
# grow unboundedly for accounts that never delete messages.
|
||||
await db.execute(
|
||||
delete(DownloadedMessageId).where(
|
||||
DownloadedMessageId.downloaded_at < cutoff_date
|
||||
)
|
||||
)
|
||||
|
||||
await db.commit()
|
||||
|
||||
logger.info(
|
||||
|
||||
@@ -4,7 +4,7 @@ import { AuthGuard } from '@/components/AuthGuard';
|
||||
import { DashboardLayout } from '@/components/DashboardLayout';
|
||||
import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query';
|
||||
import { mailAccountsApi, MailAccount } from '@/lib/api';
|
||||
import { Plus, Edit2, Trash2, CheckCircle, XCircle, AlertTriangle } from 'lucide-react';
|
||||
import { Plus, Edit2, Trash2, CheckCircle, XCircle, AlertTriangle, Power } from 'lucide-react';
|
||||
import { useState } from 'react';
|
||||
import { AddMailAccountModal } from '@/components/AddMailAccountModal';
|
||||
|
||||
@@ -25,6 +25,13 @@ export default function AccountsPage() {
|
||||
},
|
||||
});
|
||||
|
||||
const toggleMutation = useMutation({
|
||||
mutationFn: mailAccountsApi.toggle,
|
||||
onSuccess: () => {
|
||||
queryClient.invalidateQueries({ queryKey: ['mail-accounts'] });
|
||||
},
|
||||
});
|
||||
|
||||
const handleEdit = (account: MailAccount) => {
|
||||
setEditingAccount(account);
|
||||
setIsModalOpen(true);
|
||||
@@ -40,6 +47,14 @@ export default function AccountsPage() {
|
||||
}
|
||||
};
|
||||
|
||||
const handleToggle = async (id: number) => {
|
||||
try {
|
||||
await toggleMutation.mutateAsync(id);
|
||||
} catch {
|
||||
alert('Failed to update account');
|
||||
}
|
||||
};
|
||||
|
||||
const handleCloseModal = () => {
|
||||
setIsModalOpen(false);
|
||||
setEditingAccount(null);
|
||||
@@ -69,7 +84,9 @@ export default function AccountsPage() {
|
||||
{accounts.map((account) => (
|
||||
<div
|
||||
key={account.id}
|
||||
className="bg-white rounded-lg shadow-md border border-gray-200 overflow-hidden"
|
||||
className={`bg-white rounded-lg shadow-md border overflow-hidden transition-opacity ${
|
||||
account.is_enabled ? 'border-gray-200' : 'border-gray-200 opacity-60'
|
||||
}`}
|
||||
>
|
||||
<div className="p-6">
|
||||
<div className="flex items-start justify-between mb-4">
|
||||
@@ -81,9 +98,15 @@ export default function AccountsPage() {
|
||||
</div>
|
||||
<div className="flex items-center gap-2">
|
||||
{account.is_enabled ? (
|
||||
<CheckCircle className="h-5 w-5 text-green-500" aria-label="Enabled" />
|
||||
<span className="inline-flex items-center gap-1 px-2 py-0.5 rounded-full text-xs font-medium bg-green-100 text-green-700">
|
||||
<CheckCircle className="h-3 w-3" />
|
||||
Enabled
|
||||
</span>
|
||||
) : (
|
||||
<XCircle className="h-5 w-5 text-gray-400" aria-label="Disabled" />
|
||||
<span className="inline-flex items-center gap-1 px-2 py-0.5 rounded-full text-xs font-medium bg-gray-100 text-gray-500">
|
||||
<XCircle className="h-3 w-3" />
|
||||
Disabled
|
||||
</span>
|
||||
)}
|
||||
</div>
|
||||
</div>
|
||||
@@ -129,6 +152,18 @@ export default function AccountsPage() {
|
||||
)}
|
||||
|
||||
<div className="flex items-center gap-2 pt-4 border-t border-gray-200">
|
||||
<button
|
||||
onClick={() => handleToggle(account.id)}
|
||||
disabled={toggleMutation.isPending}
|
||||
title={account.is_enabled ? 'Disable account' : 'Enable account'}
|
||||
className={`flex items-center justify-center px-3 py-2 text-sm font-medium rounded-md transition-colors disabled:opacity-50 ${
|
||||
account.is_enabled
|
||||
? 'text-yellow-600 bg-yellow-50 hover:bg-yellow-100'
|
||||
: 'text-green-600 bg-green-50 hover:bg-green-100'
|
||||
}`}
|
||||
>
|
||||
<Power className="h-4 w-4" />
|
||||
</button>
|
||||
<button
|
||||
onClick={() => handleEdit(account)}
|
||||
className="flex-1 flex items-center justify-center px-3 py-2 text-sm font-medium text-blue-600 bg-blue-50 rounded-md hover:bg-blue-100 transition-colors"
|
||||
|
||||
@@ -204,6 +204,11 @@ export const mailAccountsApi = {
|
||||
return response.data;
|
||||
},
|
||||
|
||||
async toggle(id: number): Promise<MailAccount> {
|
||||
const response = await api.patch<MailAccount>(`/mail-accounts/${id}/toggle`);
|
||||
return response.data;
|
||||
},
|
||||
|
||||
async delete(id: number): Promise<void> {
|
||||
await api.delete(`/mail-accounts/${id}`);
|
||||
},
|
||||
|
||||
Reference in New Issue
Block a user