Email notification fixes

GitTimeraider · web-flow · commit 3852af3ba93f · 2025-09-01T10:54:55.000+02:00
diff --git a/app/__init__.py b/app/__init__.py
@@ -163,6 +163,38 @@ def inject_user_date_format():
     with app.app_context():
         db.create_all()
 
+        # Automatic database migration for existing installations
+        try:
+            inspector = db.inspect(db.engine)
+            
+            # Check UserSettings table for missing columns
+            user_settings_columns = [col['name'] for col in inspector.get_columns('user_settings')]
+            migrations_applied = []
+            
+            if 'last_notification_sent' not in user_settings_columns:
+                print("🔄 Auto-migrating: Adding last_notification_sent column to user_settings...")
+                db.engine.execute('ALTER TABLE user_settings ADD COLUMN last_notification_sent DATE')
+                migrations_applied.append("last_notification_sent")
+            
+            if 'notification_time' not in user_settings_columns:
+                print("🔄 Auto-migrating: Adding notification_time column to user_settings...")
+                db.engine.execute('ALTER TABLE user_settings ADD COLUMN notification_time INTEGER DEFAULT 9')
+                migrations_applied.append("notification_time")
+            
+            # Check Subscription table for missing columns
+            subscription_columns = [col['name'] for col in inspector.get_columns('subscription')]
+            
+            if 'custom_notification_days' not in subscription_columns:
+                print("🔄 Auto-migrating: Adding custom_notification_days column to subscription...")
+                db.engine.execute('ALTER TABLE subscription ADD COLUMN custom_notification_days INTEGER')
+                migrations_applied.append("custom_notification_days")
+            
+            if migrations_applied:
+                print(f"✅ Database migration completed! Added: {', '.join(migrations_applied)}")
+            
+        except Exception as e:
+            print(f"⚠️  Database migration skipped: {e}")
+
         # Create default admin user if no admin users exist
         from app.models import User, UserSettings
         admin_exists = User.query.filter_by(is_admin=True).first()
diff --git a/app/email.py b/app/email.py
@@ -176,9 +176,12 @@ def send_expiry_notification(app, user, subscriptions):
                 print("📨 Sending email...")
                 server.send_message(msg)
 
-                # Update last notification date for all subscriptions
-                for subscription in subscriptions:
-                    subscription.last_notification = datetime.now().date()
+                # Update last notification date for the user (not individual subscriptions)
+                user_settings = user.settings or UserSettings()
+                user_settings.last_notification_sent = datetime.now().date()
+                if not user.settings:
+                    user_settings.user_id = user.id
+                    db.session.add(user_settings)
                 db.session.commit()
                 
                 print(f"✅ Notification sent to {user.username} for {len(subscriptions)} subscriptions")
@@ -202,7 +205,9 @@ def send_expiry_notification(app, user, subscriptions):
 def check_expiring_subscriptions(app):
     """Check for expiring subscriptions and send notifications"""
     with app.app_context():
-        print(f"🔍 Checking expiring subscriptions at {datetime.now()}")
+        current_time = datetime.now()
+        current_hour = current_time.hour
+        print(f"🔍 Checking expiring subscriptions at {current_time} (hour: {current_hour})")
         
         # Get all users with notification settings
         users = User.query.all()
@@ -215,62 +220,62 @@ def check_expiring_subscriptions(app):
             if not user_settings.email_notifications:
                 print(f"⏭️  Skipping {user.username} - notifications disabled")
                 continue
-                
-            days_before = user_settings.notification_days
-            check_date = datetime.now().date() + timedelta(days=days_before)
+            
+            # Check if we already sent a notification today for this user
+            today = current_time.date()
+            if user_settings.last_notification_sent == today:
+                print(f"⏭️  Skipping {user.username} - already notified today")
+                continue
+            
+            # Check if it's the user's preferred notification time (±1 hour window)
+            preferred_hour = user_settings.notification_time or 9
+            if not (preferred_hour - 1 <= current_hour <= preferred_hour + 1):
+                print(f"⏭️  Skipping {user.username} - not their notification time (prefers {preferred_hour}:00, current: {current_hour}:00)")
+                continue
 
-            # Find subscriptions that are:
-            # 1. Expiring within the notification window
-            # 2. Haven't been notified today
-            # 3. Are still active
-            subscriptions = Subscription.query.filter(
-                Subscription.user_id == user.id,
-                Subscription.is_active == True,
-                Subscription.end_date.isnot(None),
-                Subscription.end_date <= check_date,
-                Subscription.end_date >= datetime.now().date(),
-                (Subscription.last_notification == None) | 
-                (Subscription.last_notification < datetime.now().date())
-            ).all()
+            # Find subscriptions expiring based on their individual or default notification days
+            expiring_subscriptions = []
+            
+            for subscription in user.subscriptions:
+                if not subscription.is_active or not subscription.end_date:
+                    continue
+                
+                # Get notification days for this specific subscription
+                notification_days = subscription.get_notification_days(user_settings)
+                check_date = today + timedelta(days=notification_days)
+                
+                # Check if this subscription is expiring within its notification window
+                if subscription.end_date <= check_date and subscription.end_date >= today:
+                    expiring_subscriptions.append(subscription)
 
-            if subscriptions:
-                print(f"📧 Sending notification to {user.username} for {len(subscriptions)} subscriptions")
-                success = send_expiry_notification(app, user, subscriptions)
+            if expiring_subscriptions:
+                print(f"📧 Sending notification to {user.username} for {len(expiring_subscriptions)} subscriptions at preferred time {preferred_hour}:00")
+                success = send_expiry_notification(app, user, expiring_subscriptions)
                 if success:
                     total_notifications += 1
                 else:
                     print(f"❌ Failed to send notification to {user.username}")
             else:
-                print(f"✅ No notifications needed for {user.username}")
+                print(f"✅ No expiring subscriptions for {user.username}")
         
         print(f"📊 Notification check completed. Sent {total_notifications} notifications.")
 
 def start_scheduler(app):
     """Start the background scheduler for checking expiring subscriptions"""
     scheduler = BackgroundScheduler()
     
-    # Check every 6 hours instead of daily for more timely notifications
+    # Check every hour to respect user-specific notification times
     scheduler.add_job(
         func=lambda: check_expiring_subscriptions(app),
         trigger="interval",
-        hours=6,
+        hours=1,
         id='check_subscriptions',
         replace_existing=True
     )
     
-    # Also add a daily job at 9 AM for primary notifications
-    scheduler.add_job(
-        func=lambda: check_expiring_subscriptions(app),
-        trigger="cron",
-        hour=9,
-        minute=0,
-        id='daily_check_subscriptions',
-        replace_existing=True
-    )
-    
     scheduler.start()
     atexit.register(lambda: scheduler.shutdown())
-    print("Email notification scheduler started")
+    print("Email notification scheduler started (checking hourly)")
 
 def send_test_email(app, user):
     """Send a test email to verify email configuration"""
diff --git a/app/forms.py b/app/forms.py
@@ -47,6 +47,9 @@ class SubscriptionForm(FlaskForm):
     payment_method_id = SelectField('Payment Method', coerce=int, validators=[Optional()])
     start_date = DateField('Start Date', validators=[DataRequired()])
     end_date = DateField('End Date (Leave blank for infinite)', validators=[Optional()])
+    custom_notification_days = IntegerField('Custom notification days (override default)', 
+                                          validators=[Optional(), NumberRange(min=1, max=365)],
+                                          render_kw={'placeholder': 'Leave blank to use default'})
     notes = TextAreaField('Notes', validators=[Optional()])
     
     def __init__(self, *args, **kwargs):
@@ -85,6 +88,10 @@ class NotificationSettingsForm(FlaskForm):
     email_notifications = BooleanField('Enable Email Notifications')
     notification_days = IntegerField('Days before expiry to send notification', 
                                    validators=[DataRequired(), NumberRange(min=1, max=365)])
+    notification_time = SelectField('Daily notification time', 
+                                  choices=[(i, f'{i:02d}:00') for i in range(24)],
+                                  coerce=int,
+                                  validators=[DataRequired()])
 
 class GeneralSettingsForm(FlaskForm):
     currency = SelectField('Preferred Display Currency', validators=[DataRequired()])
diff --git a/app/models.py b/app/models.py
@@ -28,6 +28,8 @@ class UserSettings(db.Model):
     # Notification settings
     email_notifications = db.Column(db.Boolean, default=True)
     notification_days = db.Column(db.Integer, default=7)
+    notification_time = db.Column(db.Integer, default=9)  # Hour of day (0-23) when to send notifications
+    last_notification_sent = db.Column(db.Date)  # Track when last daily summary was sent
     
     # General settings
     currency = db.Column(db.String(3), default='EUR')
@@ -128,6 +130,7 @@ class Subscription(db.Model):
     last_notification = db.Column(db.Date)
     notes = db.Column(db.Text)
     is_active = db.Column(db.Boolean, default=True)
+    custom_notification_days = db.Column(db.Integer)  # Override default notification days for this subscription
     
     # Relationships
     payment_method = db.relationship('PaymentMethod', backref='subscriptions')
@@ -220,6 +223,10 @@ def days_until_expiry(self):
         delta = self.end_date - datetime.now().date()
         return delta.days if delta.days >= 0 else 0
 
+    def get_notification_days(self, user_settings):
+        """Get effective notification days for this subscription (custom or user default)"""
+        return self.custom_notification_days if self.custom_notification_days is not None else user_settings.notification_days
+
     def get_monthly_cost_in_currency(self, target_currency):
         """Get monthly cost converted to target currency using currency converter with timeout protection"""
         from app.currency import currency_converter
diff --git a/app/routes.py b/app/routes.py
@@ -270,6 +270,7 @@ def add_subscription():
                 payment_method_id=payment_method_id,
                 start_date=form.start_date.data,
                 end_date=form.end_date.data,
+                custom_notification_days=form.custom_notification_days.data,
                 notes=form.notes.data,
                 user_id=current_user.id
             )
@@ -306,6 +307,7 @@ def edit_subscription(id):
             subscription.payment_method_id = payment_method_id
             subscription.start_date = form.start_date.data
             subscription.end_date = form.end_date.data
+            subscription.custom_notification_days = form.custom_notification_days.data
             subscription.notes = form.notes.data
             db.session.commit()
             flash('Subscription updated successfully!', 'success')
@@ -374,6 +376,7 @@ def notification_settings():
             settings = current_user.settings
         settings.email_notifications = form.email_notifications.data
         settings.notification_days = form.notification_days.data
+        settings.notification_time = form.notification_time.data
         db.session.commit()
         flash('Notification settings updated successfully!', 'success')
         return redirect(url_for('main.notification_settings'))
diff --git a/app/templates/add_subscription.html b/app/templates/add_subscription.html
@@ -157,6 +157,19 @@ <h2 class="mb-0">
                         </div>
                     </div>
                     
+                    <div class="mb-3">
+                        {{ form.custom_notification_days.label(class="form-label") }}
+                        {{ form.custom_notification_days(class="form-control") }}
+                        <small class="form-text text-muted">Override default notification days for this subscription only</small>
+                        {% if form.custom_notification_days.errors %}
+                            <div class="text-danger mt-1">
+                                {% for error in form.custom_notification_days.errors %}
+                                    <small>{{ error }}</small>
+                                {% endfor %}
+                            </div>
+                        {% endif %}
+                    </div>
+                    
                     <div class="mb-3">
                         {{ form.notes.label(class="form-label") }}
                         {{ form.notes(class="form-control", rows="3") }}
diff --git a/app/templates/edit_subscription.html b/app/templates/edit_subscription.html
@@ -172,6 +172,19 @@ <h2 class="mb-0">
                         </div>
                     </div>
                     
+                    <div class="mb-3">
+                        {{ form.custom_notification_days.label(class="form-label") }}
+                        {{ form.custom_notification_days(class="form-control") }}
+                        <small class="form-text text-muted">Override default notification days for this subscription only</small>
+                        {% if form.custom_notification_days.errors %}
+                            <div class="text-danger mt-1">
+                                {% for error in form.custom_notification_days.errors %}
+                                    <small>{{ error }}</small>
+                                {% endfor %}
+                            </div>
+                        {% endif %}
+                    </div>
+                    
                     <div class="mb-3">
                         {{ form.notes.label(class="form-label") }}
                         {{ form.notes(class="form-control", rows="3") }}
diff --git a/app/templates/notification_settings.html b/app/templates/notification_settings.html
@@ -40,6 +40,21 @@ <h2 class="mb-0">
                         {% endif %}
                     </div>
                     
+                    <div class="mb-3">
+                        {{ form.notification_time.label(class="form-label") }}
+                        {{ form.notification_time(class="form-control") }}
+                        <small class="form-text text-muted">
+                            Time of day to send daily notification emails (24-hour format)
+                        </small>
+                        {% if form.notification_time.errors %}
+                            <div class="text-danger mt-1">
+                                {% for error in form.notification_time.errors %}
+                                    <small>{{ error }}</small>
+                                {% endfor %}
+                            </div>
+                        {% endif %}
+                    </div>
+                    
                     <div class="d-flex justify-content-between">
                         <a href="{{ url_for('main.dashboard') }}" class="btn btn-secondary">
                             <i class="fas fa-arrow-left me-2"></i>Back to Dashboard
@@ -86,8 +101,8 @@ <h6 class="card-title">
                     <i class="fas fa-info-circle me-2"></i>Notification Information
                 </h6>
                 <ul class="list-unstyled mb-0">
-                    <li><i class="fas fa-check text-success me-2"></i>Notifications are checked every 6 hours</li>
-                    <li><i class="fas fa-check text-success me-2"></i>Daily notifications are sent at 9:00 AM</li>
+                    <li><i class="fas fa-check text-success me-2"></i>Notifications are checked every hour</li>
+                    <li><i class="fas fa-check text-success me-2"></i>Daily notifications are sent at your preferred time</li>
                     <li><i class="fas fa-check text-success me-2"></i>Only active subscriptions trigger notifications</li>
                     <li><i class="fas fa-check text-success me-2"></i>You won't receive duplicate notifications on the same day</li>
                     <li><i class="fas fa-paper-plane text-info me-2"></i>Use the "Send Test Email" button above to verify your email configuration</li>
