From dddb83e46cd7e7bb17201c1c2b961f4291e79922 Mon Sep 17 00:00:00 2001 From: Marcin Czachurski Date: Tue, 15 Oct 2024 12:14:28 +0200 Subject: [PATCH] Editors feature should be global --- .../Application+Configure.swift | 2 + .../Controllers/StatusesController.swift | 2 - .../Controllers/UsersController.swift | 15 ----- .../Migrations/CreateFeaturedStatuses.swift | 47 +++++++++++++++ .../Migrations/CreateFeaturedUsers.swift | 47 +++++++++++++++ .../Services/StatusesService.swift | 7 +-- .../Services/UsersService.swift | 7 +-- .../StatusesFeatureActionTests.swift | 58 +++++++++++++++++++ .../StatusesUnfeatureActionTests.swift | 26 +++++++++ .../UsersFeatureActionTests.swift | 50 +++++++++++++++- .../UsersUnfeatureActionTests.swift | 27 ++++++++- 11 files changed, 257 insertions(+), 31 deletions(-) diff --git a/Sources/VernissageServer/Application+Configure.swift b/Sources/VernissageServer/Application+Configure.swift index 1b78bf33..8c6a69bc 100644 --- a/Sources/VernissageServer/Application+Configure.swift +++ b/Sources/VernissageServer/Application+Configure.swift @@ -303,6 +303,8 @@ extension Application { self.migrations.add(TrendingHashtag.AddAmountField()) self.migrations.add(TrendingStatus.AddAmountField()) self.migrations.add(TrendingUser.AddAmountField()) + self.migrations.add(FeaturedStatus.ChangeUniqueIndex()) + self.migrations.add(FeaturedUser.ChangeUniqueIndex()) try await self.autoMigrate() } diff --git a/Sources/VernissageServer/Controllers/StatusesController.swift b/Sources/VernissageServer/Controllers/StatusesController.swift index ecd86e4b..25e88ad9 100644 --- a/Sources/VernissageServer/Controllers/StatusesController.swift +++ b/Sources/VernissageServer/Controllers/StatusesController.swift @@ -2078,7 +2078,6 @@ struct StatusesController { } if try await FeaturedStatus.query(on: request.db) - .filter(\.$user.$id == authorizationPayloadId) .filter(\.$status.$id == statusId) .first() == nil { let id = request.application.services.snowflakeService.generate() @@ -2215,7 +2214,6 @@ struct StatusesController { } if let featuredStatus = try await FeaturedStatus.query(on: request.db) - .filter(\.$user.$id == authorizationPayloadId) .filter(\.$status.$id == statusId) .first() { try await featuredStatus.delete(on: request.db) diff --git a/Sources/VernissageServer/Controllers/UsersController.swift b/Sources/VernissageServer/Controllers/UsersController.swift index 17c932fd..1ea6f342 100644 --- a/Sources/VernissageServer/Controllers/UsersController.swift +++ b/Sources/VernissageServer/Controllers/UsersController.swift @@ -1504,11 +1504,6 @@ struct UsersController { } } - - - - - /// Feature specific user. /// /// This endpoint is used to add the user to a special list of featured users. @@ -1581,7 +1576,6 @@ struct UsersController { } if try await FeaturedUser.query(on: request.db) - .filter(\.$user.$id == authorizationPayloadId) .filter(\.$featuredUser.$id == userId) .first() == nil { let id = request.application.services.snowflakeService.generate() @@ -1652,10 +1646,6 @@ struct UsersController { @Sendable func unfeature(request: Request) async throws -> UserDto { let usersService = request.application.services.usersService - - guard let authorizationPayloadId = request.userId else { - throw Abort(.forbidden) - } guard let userName = request.parameters.get("name") else { throw Abort(.badRequest) @@ -1675,7 +1665,6 @@ struct UsersController { } if let featureUser = try await FeaturedUser.query(on: request.db) - .filter(\.$user.$id == authorizationPayloadId) .filter(\.$featuredUser.$id == userId) .first() { try await featureUser.delete(on: request.db) @@ -1695,10 +1684,6 @@ struct UsersController { return userProfile } - - - - private func relationship(on request: Request, sourceId: Int64, targetUser: User) async throws -> RelationshipDto { let targetUserId = try targetUser.requireID() let relationshipsService = request.application.services.relationshipsService diff --git a/Sources/VernissageServer/Migrations/CreateFeaturedStatuses.swift b/Sources/VernissageServer/Migrations/CreateFeaturedStatuses.swift index 6a9d87d6..79886727 100644 --- a/Sources/VernissageServer/Migrations/CreateFeaturedStatuses.swift +++ b/Sources/VernissageServer/Migrations/CreateFeaturedStatuses.swift @@ -7,6 +7,7 @@ import Vapor import Fluent import SQLKit +import SQLiteKit extension FeaturedStatus { struct CreateFeaturedStatuses: AsyncMigration { @@ -40,4 +41,50 @@ extension FeaturedStatus { try await database.schema(FeaturedStatus.schema).delete() } } + + struct ChangeUniqueIndex: AsyncMigration { + func prepare(on database: Database) async throws { + // SQLite only supports adding columns in ALTER TABLE statements. + if let _ = database as? SQLiteDatabase { + return + } + + try await database + .schema(FeaturedStatus.schema) + .deleteUnique(on: "statusId", "userId") + .update() + + try await database + .schema(FeaturedStatus.schema) + .unique(on: "statusId") + .update() + + if let sqlDatabase = database as? SQLDatabase { + try await sqlDatabase + .drop(index: "\(FeaturedStatus.schema)_statusIdIndex") + .run() + + try await sqlDatabase + .drop(index: "\(FeaturedStatus.schema)_userIdIndex") + .run() + } + } + + func revert(on database: Database) async throws { + // SQLite only supports adding columns in ALTER TABLE statements. + if let _ = database as? SQLiteDatabase { + return + } + + try await database + .schema(FeaturedStatus.schema) + .deleteUnique(on: "statusId") + .update() + + try await database + .schema(FeaturedStatus.schema) + .unique(on: "statusId", "userId") + .update() + } + } } diff --git a/Sources/VernissageServer/Migrations/CreateFeaturedUsers.swift b/Sources/VernissageServer/Migrations/CreateFeaturedUsers.swift index 34a5eb1f..aedd55e5 100644 --- a/Sources/VernissageServer/Migrations/CreateFeaturedUsers.swift +++ b/Sources/VernissageServer/Migrations/CreateFeaturedUsers.swift @@ -7,6 +7,7 @@ import Vapor import Fluent import SQLKit +import SQLiteKit extension FeaturedUser { struct CreateFeaturedUsers: AsyncMigration { @@ -40,4 +41,50 @@ extension FeaturedUser { try await database.schema(FeaturedUser.schema).delete() } } + + struct ChangeUniqueIndex: AsyncMigration { + func prepare(on database: Database) async throws { + // SQLite only supports adding columns in ALTER TABLE statements. + if let _ = database as? SQLiteDatabase { + return + } + + try await database + .schema(FeaturedUser.schema) + .deleteUnique(on: "featuredUserId", "userId") + .update() + + try await database + .schema(FeaturedUser.schema) + .unique(on: "featuredUserId") + .update() + + if let sqlDatabase = database as? SQLDatabase { + try await sqlDatabase + .drop(index: "\(FeaturedUser.schema)_featuredUserIdIndex") + .run() + + try await sqlDatabase + .drop(index: "\(FeaturedUser.schema)_userIdIndex") + .run() + } + } + + func revert(on database: Database) async throws { + // SQLite only supports adding columns in ALTER TABLE statements. + if let _ = database as? SQLiteDatabase { + return + } + + try await database + .schema(FeaturedUser.schema) + .deleteUnique(on: "featuredUserId") + .update() + + try await database + .schema(FeaturedUser.schema) + .unique(on: "featuredUserId", "userId") + .update() + } + } } diff --git a/Sources/VernissageServer/Services/StatusesService.swift b/Sources/VernissageServer/Services/StatusesService.swift index c4f5c83a..b8a95a96 100644 --- a/Sources/VernissageServer/Services/StatusesService.swift +++ b/Sources/VernissageServer/Services/StatusesService.swift @@ -1338,13 +1338,8 @@ final class StatusesService: StatusesServiceType { return bookmarkedStatuses.map({ $0.$status.id }) } - private func statusIsFeatured(on request: Request, statusId: Int64) async throws -> Bool { - guard let authorizationPayloadId = request.userId else { - return false - } - + private func statusIsFeatured(on request: Request, statusId: Int64) async throws -> Bool { let amount = try await FeaturedStatus.query(on: request.db) - .filter(\.$user.$id == authorizationPayloadId) .filter(\.$status.$id == statusId) .count() diff --git a/Sources/VernissageServer/Services/UsersService.swift b/Sources/VernissageServer/Services/UsersService.swift index e4ffb73a..13ad1ad2 100644 --- a/Sources/VernissageServer/Services/UsersService.swift +++ b/Sources/VernissageServer/Services/UsersService.swift @@ -959,13 +959,8 @@ final class UsersService: UsersServiceType { return userDto } - private func userIsFeatured(on request: Request, userId: Int64) async throws -> Bool { - guard let authorizationPayloadId = request.userId else { - return false - } - + private func userIsFeatured(on request: Request, userId: Int64) async throws -> Bool { let amount = try await FeaturedUser.query(on: request.db) - .filter(\.$user.$id == authorizationPayloadId) .filter(\.$featuredUser.$id == userId) .count() diff --git a/Tests/VernissageServerTests/AcceptanceTests/StatusesController/StatusesFeatureActionTests.swift b/Tests/VernissageServerTests/AcceptanceTests/StatusesController/StatusesFeatureActionTests.swift index 334ceb18..e06e4f8d 100644 --- a/Tests/VernissageServerTests/AcceptanceTests/StatusesController/StatusesFeatureActionTests.swift +++ b/Tests/VernissageServerTests/AcceptanceTests/StatusesController/StatusesFeatureActionTests.swift @@ -46,6 +46,64 @@ extension ControllersTests { #expect(statusDto.featured == true, "Status should be marked as featured.") } + @Test("Status should be featured only once") + func statusShouldBeFeaturedOnlyOnce() async throws { + + // Arrange. + let user1 = try await application.createUser(userName: "zibifokimo") + let user2 = try await application.createUser(userName: "zicofokimo") + try await application.attach(user: user1, role: Role.moderator) + try await application.attach(user: user2, role: Role.moderator) + + let (statuses, attachments) = try await application.createStatuses(user: user1, notePrefix: "Note Featured", amount: 1) + defer { + application.clearFiles(attachments: attachments) + } + + _ = try await application.createFeaturedStatus(user: user1, status: statuses.first!) + + // Act. + _ = try application.getResponse( + as: .user(userName: "zicofokimo", password: "p@ssword"), + to: "/statuses/\(statuses.first!.requireID())/feature", + method: .POST, + decodeTo: StatusDto.self + ) + + // Assert. + let allFeaturedStatuses = try await application.getAllFeaturedStatuses() + #expect(allFeaturedStatuses.count { $0.status.id == statuses.first!.id } == 1, "Status wasn't featured once.") + } + + @Test("Status should be mark as featured even if other moderator featured status") + func statusShouldBeFeaturedEvenIfOtherModeratorFeaturedStatus() async throws { + + // Arrange. + let user1 = try await application.createUser(userName: "andyfokimo") + let user2 = try await application.createUser(userName: "arrinfokimo") + try await application.attach(user: user1, role: Role.moderator) + try await application.attach(user: user2, role: Role.moderator) + + let (statuses, attachments) = try await application.createStatuses(user: user1, notePrefix: "Note Featured", amount: 1) + defer { + application.clearFiles(attachments: attachments) + } + + _ = try await application.createFeaturedStatus(user: user1, status: statuses.first!) + + // Act. + let statusDto = try application.getResponse( + as: .user(userName: "arrinfokimo", password: "p@ssword"), + to: "/statuses/\(statuses.first!.requireID())", + method: .GET, + decodeTo: UserDto.self + ) + + // Assert. + #expect(statusDto.id != nil, "Status wasn't returned.") + #expect(statusDto.featured == true, "Status should be marked as featured.") + } + @Test("Forbidden should be returned for regular user") func forbiddenShouldbeReturnedForRegularUser() async throws { diff --git a/Tests/VernissageServerTests/AcceptanceTests/StatusesController/StatusesUnfeatureActionTests.swift b/Tests/VernissageServerTests/AcceptanceTests/StatusesController/StatusesUnfeatureActionTests.swift index 15ab8a05..11340102 100644 --- a/Tests/VernissageServerTests/AcceptanceTests/StatusesController/StatusesUnfeatureActionTests.swift +++ b/Tests/VernissageServerTests/AcceptanceTests/StatusesController/StatusesUnfeatureActionTests.swift @@ -47,6 +47,32 @@ extension ControllersTests { #expect(statusDto.featured == false, "Status should be marked as unfeatured.") } + @Test("Status should be unfeatured even if other moderator feature status") + func statusShouldBeUnfeaturedEvenIfOtherModeratorFeatureStatus() async throws { + + // Arrange. + let user1 = try await application.createUser(userName: "zibirojon") + let user2 = try await application.createUser(userName: "zicorojon") + let (statuses, attachments) = try await application.createStatuses(user: user1, notePrefix: "Note Unfavorited", amount: 1) + defer { + application.clearFiles(attachments: attachments) + } + _ = try await application.createFeaturedStatus(user: user1, status: statuses.first!) + try await application.attach(user: user2, role: Role.moderator) + + // Act. + _ = try application.getResponse( + as: .user(userName: "zicorojon", password: "p@ssword"), + to: "/statuses/\(statuses.first!.requireID())/unfeature", + method: .POST, + decodeTo: StatusDto.self + ) + + // Assert. + let allFeaturedStatuses = try await application.getAllFeaturedStatuses() + #expect(allFeaturedStatuses.contains { $0.status.id == statuses.first!.id } == false, "Status wasn't unfeatured.") + } + @Test("Forbidden should be returned for regular user") func forbiddenShouldbeReturnedForRegularUser() async throws { diff --git a/Tests/VernissageServerTests/AcceptanceTests/UsersController/UsersFeatureActionTests.swift b/Tests/VernissageServerTests/AcceptanceTests/UsersController/UsersFeatureActionTests.swift index 1d43fc10..d770bb02 100644 --- a/Tests/VernissageServerTests/AcceptanceTests/UsersController/UsersFeatureActionTests.swift +++ b/Tests/VernissageServerTests/AcceptanceTests/UsersController/UsersFeatureActionTests.swift @@ -37,7 +37,55 @@ extension ControllersTests { ) // Assert. - #expect(userDto.id != nil, "User wasn't featured.") + #expect(userDto.id != nil, "User wasn't returned.") + #expect(userDto.featured == true, "User should be marked as featured.") + } + + @Test("User should be featured only once") + func userShouldBeFeaturedOnlyOnce() async throws { + + // Arrange. + let user1 = try await application.createUser(userName: "nicoleborin") + let user2 = try await application.createUser(userName: "vikiborin") + let user3 = try await application.createUser(userName: "franborin") + try await application.attach(user: user1, role: Role.moderator) + try await application.attach(user: user2, role: Role.moderator) + _ = try await application.createFeaturedUser(user: user1, featuredUser: user3) + + // Act. + _ = try application.getResponse( + as: .user(userName: "vikiborin", password: "p@ssword"), + to: "/users/@\(user3.userName)/feature", + method: .POST, + decodeTo: UserDto.self + ) + + // Assert. + let allFeaturedUsers = try await application.getAllFeaturedUsers() + #expect(allFeaturedUsers.count { $0.featuredUser.id == user3.id } == 1, "User wasn't featured once.") + } + + @Test("User should be mark as featured even if other moderator featured user") + func userShouldBeFeaturedEvenIfOtherModeratorFeaturedUser() async throws { + + // Arrange. + let user1 = try await application.createUser(userName: "zibiborin") + let user2 = try await application.createUser(userName: "zackborin") + let user3 = try await application.createUser(userName: "zomoborin") + try await application.attach(user: user1, role: Role.moderator) + try await application.attach(user: user2, role: Role.moderator) + _ = try await application.createFeaturedUser(user: user1, featuredUser: user3) + + // Act. + let userDto = try application.getResponse( + as: .user(userName: "zackborin", password: "p@ssword"), + to: "/users/@\(user3.userName)", + method: .GET, + decodeTo: UserDto.self + ) + + // Assert. + #expect(userDto.id != nil, "User wasn't returned.") #expect(userDto.featured == true, "User should be marked as featured.") } diff --git a/Tests/VernissageServerTests/AcceptanceTests/UsersController/UsersUnfeatureActionTests.swift b/Tests/VernissageServerTests/AcceptanceTests/UsersController/UsersUnfeatureActionTests.swift index a28d16e2..6d8e58cf 100644 --- a/Tests/VernissageServerTests/AcceptanceTests/UsersController/UsersUnfeatureActionTests.swift +++ b/Tests/VernissageServerTests/AcceptanceTests/UsersController/UsersUnfeatureActionTests.swift @@ -39,10 +39,35 @@ extension ControllersTests { ) // Assert. - #expect(userDto.id != nil, "User wasn't unfeatured.") + #expect(userDto.id != nil, "User wasn't returned.") #expect(userDto.featured == false, "User should be marked as unfeatured.") } + @Test("User should be unfeatured even if other moderator feature user") + func userShouldBeUnfeaturedEvenIfOtherModeratorFeatureUser() async throws { + + // Arrange. + let user1 = try await application.createUser(userName: "chrisgupok") + let user2 = try await application.createUser(userName: "rickgupok") + let user3 = try await application.createUser(userName: "trokgupok") + + try await application.attach(user: user1, role: Role.moderator) + try await application.attach(user: user2, role: Role.moderator) + _ = try await application.createFeaturedUser(user: user1, featuredUser: user3) + + // Act. + _ = try application.getResponse( + as: .user(userName: "rickgupok", password: "p@ssword"), + to: "/users/@\(user3.userName)/unfeature", + method: .POST, + decodeTo: UserDto.self + ) + + // Assert. + let allFeaturedUsers = try await application.getAllFeaturedUsers() + #expect(allFeaturedUsers.contains { $0.featuredUser.id == user3.id } == false, "User wasn't unfeatured.") + } + @Test("Forbidden should be returned for regular user") func forbiddenShouldbeReturnedForRegularUser() async throws {