From a92a22cd4168f3c98cf99f90667b032ebbae7663 Mon Sep 17 00:00:00 2001 From: Volodymyr Chekyrta <127732735+volodymyr-chekyrta@users.noreply.github.com> Date: Fri, 15 Sep 2023 18:34:43 +0300 Subject: [PATCH] Codestyle improvements (#76) * codestyle improvements * remove unused code --- .../Core/Extensions/CollectionExtension.swift | 2 +- .../Data/Network/DashboardEndpoint.swift | 8 +- .../Presentation/Posts/PostsView.swift | 21 ++- .../Presentation/Posts/PostsViewModel.swift | 152 ++++-------------- .../Posts/PostViewModelTests.swift | 26 +-- 5 files changed, 58 insertions(+), 151 deletions(-) diff --git a/Core/Core/Extensions/CollectionExtension.swift b/Core/Core/Extensions/CollectionExtension.swift index 73fbeba15..ba2ff088a 100644 --- a/Core/Core/Extensions/CollectionExtension.swift +++ b/Core/Core/Extensions/CollectionExtension.swift @@ -7,7 +7,7 @@ import Foundation -extension Collection { +public extension Collection { /// Returns the element at the specified index if it is within bounds, otherwise nil. subscript (safe index: Index) -> Element? { return indices.contains(index) ? self[index] : nil diff --git a/Dashboard/Dashboard/Data/Network/DashboardEndpoint.swift b/Dashboard/Dashboard/Data/Network/DashboardEndpoint.swift index 02903fab6..d9e4dec06 100644 --- a/Dashboard/Dashboard/Data/Network/DashboardEndpoint.swift +++ b/Dashboard/Dashboard/Data/Network/DashboardEndpoint.swift @@ -11,25 +11,25 @@ import Alamofire enum DashboardEndpoint: EndPointType { case getMyCourses(username: String, page: Int) - + var path: String { switch self { case let .getMyCourses(username, _): return "/mobile_api_extensions/v1/users/\(username)/course_enrollments" } } - + var httpMethod: HTTPMethod { switch self { case .getMyCourses: return .get } } - + var headers: HTTPHeaders? { nil } - + var task: HTTPTask { switch self { case let .getMyCourses(_, page): diff --git a/Discussion/Discussion/Presentation/Posts/PostsView.swift b/Discussion/Discussion/Presentation/Posts/PostsView.swift index d88441939..dc7145a3a 100644 --- a/Discussion/Discussion/Presentation/Posts/PostsView.swift +++ b/Discussion/Discussion/Presentation/Posts/PostsView.swift @@ -14,7 +14,6 @@ public struct PostsView: View { @ObservedObject private var viewModel: PostsViewModel @State private var isShowProgress: Bool = true @State private var showingAlert = false - @State private var listAnimation: Animation? private let router: DiscussionRouter private let title: String private let currentBlockID: String @@ -55,7 +54,6 @@ public struct PostsView: View { HStack { Group { Button(action: { - listAnimation = .easeIn viewModel.generateButtons(type: .filter) showingAlert = true }, label: { @@ -64,7 +62,6 @@ public struct PostsView: View { }) Spacer() Button(action: { - listAnimation = .easeIn viewModel.generateButtons(type: .sort) showingAlert = true }, label: { @@ -84,10 +81,8 @@ public struct PostsView: View { } .frameLimit() RefreshableScrollViewCompat(action: { - listAnimation = nil viewModel.resetPosts() _ = await viewModel.getPosts( - courseID: courseID, pageNumber: 1, withProgress: false ) @@ -134,7 +129,6 @@ public struct PostsView: View { .onAppear { Task { await viewModel.getPostsPagination( - courseID: self.courseID, index: index ) } @@ -179,7 +173,7 @@ public struct PostsView: View { } } }.frameLimit() - .animation(listAnimation) + .animation(nil) .onRightSwipeGesture { router.back() } @@ -197,7 +191,10 @@ public struct PostsView: View { } .onFirstAppear { Task { - await viewModel.getPosts(courseID: courseID, pageNumber: 1, withProgress: true) + await viewModel.getPosts( + pageNumber: 1, + withProgress: true + ) } } .navigationBarHidden(!showTopMenu) @@ -216,11 +213,11 @@ public struct PostsView: View { @MainActor private func reloadPage(onSuccess: @escaping () -> Void) { Task { - listAnimation = nil viewModel.resetPosts() - _ = await viewModel.getPosts(courseID: courseID, - pageNumber: 1, - withProgress: false) + _ = await viewModel.getPosts( + pageNumber: 1, + withProgress: false + ) onSuccess() } } diff --git a/Discussion/Discussion/Presentation/Posts/PostsViewModel.swift b/Discussion/Discussion/Presentation/Posts/PostsViewModel.swift index f8baeba06..baad91ffc 100644 --- a/Discussion/Discussion/Presentation/Posts/PostsViewModel.swift +++ b/Discussion/Discussion/Presentation/Posts/PostsViewModel.swift @@ -28,7 +28,7 @@ public class PostsViewModel: ObservableObject { public var nextPage = 1 public var totalPages = 1 - public private(set) var fetchInProgress = false + @Published public private(set) var fetchInProgress = false public enum ButtonType { case sort @@ -43,7 +43,7 @@ public class PostsViewModel: ObservableObject { if let courseID { resetPosts() Task { - _ = await getPosts(courseID: courseID, pageNumber: 1) + _ = await getPosts(pageNumber: 1) } } } @@ -53,7 +53,7 @@ public class PostsViewModel: ObservableObject { if let courseID { resetPosts() Task { - _ = await getPosts(courseID: courseID, pageNumber: 1) + _ = await getPosts(pageNumber: 1) } } } @@ -109,9 +109,6 @@ public class PostsViewModel: ObservableObject { } public func resetPosts() { - filteredPosts = [] - discussionPosts = [] - threads.threads = [] nextPage = 1 totalPages = 1 } @@ -170,134 +167,36 @@ public class PostsViewModel: ObservableObject { } @MainActor - func getPostsPagination(courseID: String, index: Int, withProgress: Bool = true) async { - if !fetchInProgress { - if totalPages > 1 { - if index == filteredPosts.count - 3 { - if totalPages != 1 { - if nextPage <= totalPages { - _ = await getPosts( - courseID: courseID, - pageNumber: self.nextPage, - withProgress: withProgress - ) - } - } - } - } + func getPostsPagination(index: Int, withProgress: Bool = true) async { + guard !fetchInProgress else { return } + if totalPages > 1, index >= filteredPosts.count - 3, nextPage <= totalPages { + _ = await getPosts( + pageNumber: self.nextPage, + withProgress: withProgress + ) } } - // swiftlint:disable function_body_length @MainActor - public func getPosts(courseID: String, pageNumber: Int, withProgress: Bool = true) async -> Bool { + public func getPosts(pageNumber: Int, withProgress: Bool = true) async -> Bool { fetchInProgress = true isShowProgress = withProgress do { if pageNumber == 1 { - switch type { - case .allPosts: - threads.threads = try await interactor - .getThreadsList(courseID: courseID, - type: .allPosts, - sort: sortTitle, - filter: filterTitle, - page: pageNumber).threads - if threads.threads.indices.contains(0) { - self.totalPages = threads.threads[0].numPages - self.nextPage = 2 - } - case .followingPosts: - threads.threads = try await interactor - .getThreadsList(courseID: courseID, - type: .followingPosts, - sort: sortTitle, - filter: filterTitle, - page: pageNumber).threads - if threads.threads.indices.contains(0) { - self.totalPages = threads.threads[0].numPages - self.nextPage = 2 - } - case .nonCourseTopics: - threads.threads = try await interactor - .getThreadsList(courseID: courseID, - type: .nonCourseTopics, - sort: sortTitle, - filter: filterTitle, - page: pageNumber).threads - if threads.threads.indices.contains(0) { - self.totalPages = threads.threads[0].numPages - self.nextPage = 2 - } - case .courseTopics(topicID: let topicID): - threads.threads = try await interactor - .getThreadsList(courseID: courseID, - type: .courseTopics(topicID: topicID), - sort: sortTitle, - filter: filterTitle, - page: pageNumber).threads - if threads.threads.indices.contains(0) { - self.totalPages = threads.threads[0].numPages - self.nextPage = 2 - } - case .none: - isShowProgress = false - return false + threads.threads = try await getThreadsList(type: type, page: pageNumber) + if threads.threads.indices.contains(0) { + totalPages = threads.threads[0].numPages + nextPage = 2 } } else { - switch type { - case .allPosts: - threads.threads += try await interactor - .getThreadsList(courseID: courseID, - type: .allPosts, - sort: sortTitle, - filter: filterTitle, - page: pageNumber).threads - if threads.threads.indices.contains(0) { - self.totalPages = threads.threads[0].numPages - self.nextPage += 1 - } - case .followingPosts: - threads.threads += try await interactor - .getThreadsList(courseID: courseID, - type: .followingPosts, - sort: sortTitle, - filter: filterTitle, - page: pageNumber).threads - if threads.threads.indices.contains(0) { - self.totalPages = threads.threads[0].numPages - self.nextPage += 1 - } - case .nonCourseTopics: - threads.threads += try await interactor - .getThreadsList(courseID: courseID, - type: .nonCourseTopics, - sort: sortTitle, - filter: filterTitle, - page: pageNumber).threads - if threads.threads.indices.contains(0) { - self.totalPages = threads.threads[0].numPages - self.nextPage += 1 - } - case .courseTopics(topicID: let topicID): - threads.threads += try await interactor - .getThreadsList(courseID: courseID, - type: .courseTopics(topicID: topicID), - sort: sortTitle, - filter: filterTitle, - page: pageNumber).threads - if threads.threads.indices.contains(0) { - self.totalPages = threads.threads[0].numPages - self.nextPage += 1 - } - case .none: - isShowProgress = false - return false + threads.threads += try await getThreadsList(type: type, page: pageNumber) + if threads.threads.indices.contains(0) { + totalPages = threads.threads[0].numPages + nextPage += 1 } } discussionPosts = generatePosts(threads: threads) filteredPosts = discussionPosts - self.filteredPosts = self.discussionPosts isShowProgress = false fetchInProgress = false return true @@ -312,7 +211,18 @@ public class PostsViewModel: ObservableObject { return false } } - // swiftlint:enable function_body_length + + @MainActor + private func getThreadsList(type: ThreadType, page: Int) async throws -> [UserThread] { + guard let courseID else { return [] } + return try await interactor.getThreadsList( + courseID: courseID, + type: type, + sort: sortTitle, + filter: filterTitle, + page: page + ).threads + } private func updateUnreadCommentsCount(id: String) { var threads = threads.threads diff --git a/Discussion/DiscussionTests/Presentation/Posts/PostViewModelTests.swift b/Discussion/DiscussionTests/Presentation/Posts/PostViewModelTests.swift index cd4dbab3c..d079c8944 100644 --- a/Discussion/DiscussionTests/Presentation/Posts/PostViewModelTests.swift +++ b/Discussion/DiscussionTests/Presentation/Posts/PostViewModelTests.swift @@ -108,33 +108,30 @@ final class PostViewModelTests: XCTestCase { var result = false let viewModel = PostsViewModel(interactor: interactor, router: router, config: config) + viewModel.courseID = "1" viewModel.type = .allPosts Given(interactor, .getThreadsList(courseID: .any, type: .any, sort: .any, filter: .any, page: .any, willReturn: threads)) viewModel.type = .allPosts - result = await viewModel.getPosts(courseID: "1", pageNumber: 1) + result = await viewModel.getPosts(pageNumber: 1) XCTAssertTrue(result) result = false viewModel.type = .courseTopics(topicID: "") - result = await viewModel.getPosts(courseID: "1", pageNumber: 1) + result = await viewModel.getPosts(pageNumber: 1) XCTAssertTrue(result) result = false viewModel.type = .followingPosts - result = await viewModel.getPosts(courseID: "1", pageNumber: 1) + result = await viewModel.getPosts(pageNumber: 1) XCTAssertTrue(result) result = false viewModel.type = .nonCourseTopics - result = await viewModel.getPosts(courseID: "1", pageNumber: 1) + result = await viewModel.getPosts(pageNumber: 1) XCTAssertTrue(result) result = false - - viewModel.type = .none - result = await viewModel.getPosts(courseID: "1", pageNumber: 1) - XCTAssertFalse(result) Verify(interactor, 4, .getThreadsList(courseID: .value("1"), type: .any, sort: .any, filter: .any, page: .value(1))) @@ -154,8 +151,9 @@ final class PostViewModelTests: XCTestCase { Given(interactor, .getThreadsList(courseID: .any, type: .any, sort: .any, filter: .any, page: .any, willThrow: noInternetError)) + viewModel.courseID = "1" viewModel.type = .allPosts - result = await viewModel.getPosts(courseID: "1", pageNumber: 1) + result = await viewModel.getPosts(pageNumber: 1) Verify(interactor, 1, .getThreadsList(courseID: .any, type: .any, sort: .any, filter: .any, page: .any)) @@ -174,8 +172,9 @@ final class PostViewModelTests: XCTestCase { Given(interactor, .getThreadsList(courseID: .any, type: .any, sort: .any, filter: .any, page: .any, willThrow: NSError())) + viewModel.courseID = "1" viewModel.type = .allPosts - result = await viewModel.getPosts(courseID: "1", pageNumber: 1) + result = await viewModel.getPosts(pageNumber: 1) Verify(interactor, 1, .getThreadsList(courseID: .any, type: .any, sort: .any, filter: .any, page: .any)) @@ -193,9 +192,10 @@ final class PostViewModelTests: XCTestCase { Given(interactor, .getThreadsList(courseID: .any, type: .any, sort: .any, filter: .any, page: .any, willReturn: threads)) + viewModel.courseID = "1" viewModel.type = .allPosts viewModel.sortTitle = .mostActivity - _ = await viewModel.getPosts(courseID: "1", pageNumber: 1) + _ = await viewModel.getPosts(pageNumber: 1) XCTAssertTrue(viewModel.filteredPosts[0].title == "1") Given(interactor, .getThreadsList(courseID: .any, type: .any, sort: .value(.recentActivity), filter: .any, page: .any, @@ -203,7 +203,7 @@ final class PostViewModelTests: XCTestCase { viewModel.filterTitle = .unread viewModel.sortTitle = .recentActivity - _ = await viewModel.getPosts(courseID: "1", pageNumber: 1) + _ = await viewModel.getPosts(pageNumber: 1) XCTAssertTrue(viewModel.filteredPosts[0].title == "1") XCTAssertNotNil(viewModel.filteredPosts.first(where: {$0.unreadCommentCount == 4})) @@ -212,7 +212,7 @@ final class PostViewModelTests: XCTestCase { viewModel.filterTitle = .unanswered viewModel.sortTitle = .mostVotes - _ = await viewModel.getPosts(courseID: "1", pageNumber: 1) + _ = await viewModel.getPosts(pageNumber: 1) XCTAssertTrue(viewModel.filteredPosts[0].title == "1") XCTAssertNotNil(viewModel.filteredPosts.first(where: { $0.hasEndorsed }))