Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a banner to ask user to validate their email #24172

Merged
merged 8 commits into from
Mar 20, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions WordPress/Classes/ViewRelated/Me/Me Main/MeViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ class MeViewController: UITableViewController {
}

ImmuTable.registerRows([
VerifyEmailRow.self,
NavigationItemRow.self,
IndicatorNavigationItemRow.self,
ButtonRow.self,
Expand All @@ -47,6 +48,7 @@ class MeViewController: UITableViewController {
WPStyleGuide.configureAutomaticHeightRows(for: tableView)

NotificationCenter.default.addObserver(self, selector: #selector(MeViewController.accountDidChange), name: NSNotification.Name.WPAccountDefaultWordPressComAccountChanged, object: nil)
NotificationCenter.default.addObserver(self, selector: #selector(MeViewController.refreshAccountDetailsAndSettings), name: UIApplication.didBecomeActiveNotification, object: nil)

WPStyleGuide.configureColors(view: view, tableView: tableView)
tableView.accessibilityIdentifier = "Me Table"
Expand Down Expand Up @@ -114,9 +116,13 @@ class MeViewController: UITableViewController {

fileprivate func tableViewModel(with account: WPAccount?) -> ImmuTable {
let accessoryType: UITableViewCell.AccessoryType = .disclosureIndicator

let loggedIn = account != nil

var verificationSection: ImmuTableSection?
if let account, account.verificationStatus == .unverified {
verificationSection = ImmuTableSection(rows: [VerifyEmailRow()])
}

let myProfile = NavigationItemRow(
title: RowTitles.myProfile,
icon: UIImage(named: "site-menu-people")?.withRenderingMode(.alwaysTemplate),
Expand Down Expand Up @@ -162,7 +168,14 @@ class MeViewController: UITableViewController {

let shouldShowQRLoginRow = AppConfiguration.qrLoginEnabled && !(account?.settings?.twoStepEnabled ?? false)

var sections: [ImmuTableSection] = [
var sections: [ImmuTableSection] = []

// Add verification section first if it exists
if let verificationSection {
sections.append(verificationSection)
}

sections.append(contentsOf: [
ImmuTableSection(rows: {
var rows: [ImmuTableRow] = [appSettingsRow]
if loggedIn {
Expand All @@ -176,7 +189,7 @@ class MeViewController: UITableViewController {
return rows
}()),
ImmuTableSection(rows: [helpAndSupportIndicator]),
]
])

#if IS_JETPACK
if RemoteFeatureFlag.domainManagement.enabled() && loggedIn && !isSidebarModeEnabled {
Expand Down Expand Up @@ -405,6 +418,7 @@ class MeViewController: UITableViewController {
}
return false
}

guard let sections = handler?.viewModel.sections,
let section = sections.firstIndex(where: { $0.rows.contains(where: matchRow) }),
let row = sections[section].rows.firstIndex(where: matchRow) else {
Expand Down Expand Up @@ -432,7 +446,7 @@ class MeViewController: UITableViewController {
return try? WPAccount.lookupDefaultWordPressComAccount(in: ContextManager.shared.mainContext)
}

fileprivate func refreshAccountDetailsAndSettings() {
@objc fileprivate func refreshAccountDetailsAndSettings() {
guard let account = defaultAccount(), let api = account.wordPressComRestApi else {
reloadViewModel()
return
Expand Down
230 changes: 230 additions & 0 deletions WordPress/Classes/ViewRelated/Me/Me Main/VerifyEmailRow.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,230 @@
import Foundation
import UIKit
import SwiftUI
import WordPressUI
import Combine

final class VerifyEmailRow: ImmuTableRow {
static let cell = ImmuTableCell.class(VerifyEmailCell.self)
let action: ImmuTableAction? = nil

func configureCell(_ cell: UITableViewCell) {
// Do nothing.
}
}

final class VerifyEmailCell: UITableViewCell {
private let hostingView: UIHostingView<VerifyEmailView>

override init(style: UITableViewCell.CellStyle, reuseIdentifier: String?) {
hostingView = .init(view: .init())
super.init(style: style, reuseIdentifier: reuseIdentifier)
setupView()
}

required init?(coder: NSCoder) {
fatalError("init(coder:) has not been implemented")
}

private func setupView() {
selectionStyle = .none
backgroundColor = .systemRed.withAlphaComponent(0.9)

contentView.addSubview(hostingView)
hostingView.translatesAutoresizingMaskIntoConstraints = false

NSLayoutConstraint.activate([
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use extensions from WordPressUI:

contentView.addSubview(hostingView)
hostingView.pinEdges(to: contentView)

hostingView.translatesAutoresizingMaskIntoConstraints will not be needed if you do.

hostingView.topAnchor.constraint(equalTo: contentView.topAnchor),
hostingView.leadingAnchor.constraint(equalTo: contentView.leadingAnchor),
hostingView.trailingAnchor.constraint(equalTo: contentView.trailingAnchor),
hostingView.bottomAnchor.constraint(equalTo: contentView.bottomAnchor)
])
}

}

private struct VerifyEmailView: View {
@StateObject var viewModel: VerifyEmailViewModel = .init()
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
@StateObject var viewModel: VerifyEmailViewModel = .init()
@StateObject private var viewModel = VerifyEmailViewModel()


var body: some View {
VStack(alignment: .leading, spacing: 4) {
Text(viewModel.state.title)
.font(.subheadline.weight(.semibold))
.foregroundColor(.white)

Text(viewModel.state.message)
.font(.callout)
.foregroundColor(.white)

Spacer()

if case .sent = viewModel.state {
HStack(spacing: 4) {
Image(systemName: "checkmark.circle.fill")
.foregroundColor(.white)
Text(Strings.verificationSent)
.font(.callout)
.foregroundColor(.white)
}
.frame(maxWidth: .infinity, alignment: .center)
} else {
HStack {
Spacer()

Button {
viewModel.sendVerificationEmail()
} label: {
HStack {
if viewModel.state.showsActivityIndicator {
ProgressView()
.progressViewStyle(CircularProgressViewStyle())
}

Text(viewModel.state.buttonTitle)
.foregroundColor(.black)
}
.padding(.horizontal, 16)
.padding(.vertical, 8)
.background(
RoundedRectangle(cornerRadius: 8)
.fill(.white)
)
}
.buttonStyle(.plain)
.disabled(!viewModel.state.isButtonEnabled)

Spacer()
}
}
}
.padding(16)
.frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .topLeading)
}
}

// This value is not an actual "timeout" value of the verification link. It's just an arbitrary value to prevent
// users from sending links repeatedly.
private let verificationLinkTimeout: TimeInterval = 300
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems a bit on a high end. I'd go with 60 seconds, perhaps?

I'd suggest showing the timer in the UI.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think showing a countdown on the UI is not necessary? Showing a countdown feels like it's an action that the user needs to take action right now, but there is no real urgency in clicking the email link.

Copy link
Contributor

Choose a reason for hiding this comment

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

The app could start showing it if you tap "Resend" before it's possible. This way you'll know when you can tap it again. If that's what it already does – please ignore it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The app could start showing it if you tap "Resend" before it's possible.

There are no real restrictions on the endpoint to limit when sending another email is possible.

BTW, I checked the website UI. It neither show a timer nor prevent users from keep resending emails. Once user refresh the webpage, they can send another one.

I pushed another commit to follow the web UI.

Start Sent
start sent
Start Sent
desktop-start

Copy link
Contributor

Choose a reason for hiding this comment

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

It neither show a timer nor prevent users from keep resending emails.

Well, if it's relatively free, it's should be fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I'll keep it the same with the web for now.


@MainActor
private class VerifyEmailViewModel: ObservableObject {
enum State {
case needsVerification
case sending
case sent(Date)
case error(Error)

var title: String {
Strings.title
}

var message: String {
let email = try? WPAccount.lookupDefaultWordPressComAccount(in: ContextManager.shared.mainContext)?.email ?? ""

switch self {
case .needsVerification, .sending:
if let email, !email.isEmpty {
return String(format: Strings.verifyMessage, email)
} else {
return Strings.verifyMessageNoEmail
}

case .sent:
if let email, !email.isEmpty {
return String(format: Strings.sentMessage, email)
} else {
return Strings.sentMessageNoEmail
}

case .error(let error):
return error.localizedDescription
}
}

var buttonTitle: String {
switch self {
case .needsVerification:
return Strings.sendButton
case .sending:
return Strings.sendingButton
case .sent:
return Strings.sentButton
case .error:
return Strings.retryButton
}
}

var isButtonEnabled: Bool {
switch self {
case .needsVerification, .error: return true
case .sending: return false
case .sent(let date):
return Date().timeIntervalSince(date) >= verificationLinkTimeout
}
}

var showsActivityIndicator: Bool {
if case .sending = self {
return true
}
return false
}
}

private let userID: NSNumber

private var lastVerificationSentDate: Date? {
get {
let key = "LastEmailVerificationSentDate-\(userID)"
return UserDefaults.standard.object(forKey: key) as? Date
}
set {
let key = "LastEmailVerificationSentDate-\(userID)"
UserDefaults.standard.set(newValue, forKey: key)
}
}

@Published private(set) var state: State

init() {
userID = (try? WPAccount.lookupDefaultWordPressComAccount(in: ContextManager.shared.mainContext)?.userID) ?? 0
state = .needsVerification

if let sentDate = lastVerificationSentDate,
Date().timeIntervalSince(sentDate) < verificationLinkTimeout {
state = .sent(sentDate)
}
}

func sendVerificationEmail() {
guard state.isButtonEnabled else { return }

state = .sending

let accountService = AccountService(coreDataStack: ContextManager.sharedInstance())
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
let accountService = AccountService(coreDataStack: ContextManager.sharedInstance())
let accountService = AccountService(coreDataStack: ContextManager.shared)

accountService.requestVerificationEmail({ [weak self] in
Task { @MainActor [weak self] in
guard let self else { return }
self.lastVerificationSentDate = Date()
self.state = .sent(Date())
}
}, failure: { [weak self] error in
Task { @MainActor [weak self] in
self?.state = .error(error)
}
})
}
}

private enum Strings {
static let title = NSLocalizedString("me.verifyEmail.title", value: "Verify Your Email", comment: "Title for email verification card")
static let verifyMessage = NSLocalizedString("me.verifyEmail.message.withEmail", value: "Please verify your email address (%@) to unlock all features.", comment: "Message for email verification card with email address")
static let verifyMessageNoEmail = NSLocalizedString("me.verifyEmail.message.noEmail", value: "Please verify your email address to unlock all features.", comment: "Message for email verification card")
static let sentMessage = NSLocalizedString("me.verifyEmail.sent.message.withEmail", value: "We've sent a verification link to %@. Please check your inbox and click the link.", comment: "Message shown after verification link is sent with email address")
static let sentMessageNoEmail = NSLocalizedString("me.verifyEmail.sent.message.noEmail", value: "We've sent a verification link to your email address. Please check your inbox and click the link.", comment: "Message shown after verification link is sent")
static let sendButton = NSLocalizedString("me.verifyEmail.button.send", value: "Send Verification Link", comment: "Button title to send verification link")
static let sendingButton = NSLocalizedString("me.verifyEmail.button.sending", value: "Sending...", comment: "Button title while verification link is being sent")
static let sentButton = NSLocalizedString("me.verifyEmail.button.sent", value: "Link Sent!", comment: "Button title after verification link is sent")
static let retryButton = NSLocalizedString("me.verifyEmail.button.retry", value: "Try Again", comment: "Button title when verification link sending failed")
static let verificationSent = NSLocalizedString("me.verifyEmail.status.sent", value: "Verification link sent", comment: "Message shown when verification link has been sent")
}