Allow one sponsoring to target multiple teams #264

sibbl · 2022-05-16T10:06:13Z

Work in progress

sibbl · 2022-05-16T10:07:19Z

src/main/java/backend/controller/SponsoringController.kt

@@ -89,41 +92,49 @@ class SponsoringController(private var sponsoringService: SponsoringService,
    @PreAuthorize("isAuthenticated()")
    @PostMapping("/event/{eventId}/team/{teamId}/sponsoring/")
    @ResponseStatus(CREATED)
-    fun createSponsoring(@PathVariable teamId: Long,
+    fun createSponsoring(@PathVariable eventId: Long,


For backwords compatibility, I'd personally like to keep this endpoint. It will accept a sponsoring be created for a single team.

We do have unit tests for it, so just adding a new endpoint in the first place should be the solution.

sibbl · 2022-05-16T10:08:27Z

src/main/java/backend/model/event/Team.kt

@@ -201,7 +201,12 @@ class Team : BasicEntity, Blockable {
    }

    private fun Sponsoring.toEmailListing(): String {
-        return "<b>Name</b> ${this.sponsor?.firstname} ${this.sponsor?.lastname} <b>Status</b> ${this.status} <b>Betrag pro km</b> ${this.amountPerKm.display()} <b>Limit</b> ${this.limit.display()} <b>Gereiste KM</b> ${this.team?.getCurrentDistance()} <b>Spendenversprechen</b> ${this.billableAmount().display()}"
+        var result = "<b>Name</b> ${this.sponsor?.firstname} ${this.sponsor?.lastname} <b>Status</b> ${this.status} <b>Betrag pro km</b> ${this.amountPerKm.display()} <b>Limit</b> ${this.limit.display()} <b>Spendenversprechen</b> ${this.billableAmount().display()}"


We need to check whether the visualization is correct this way.

We should also add a ticket for HTML encoding the content. Team names can include any HTML which is displayed here...

sibbl · 2022-05-16T10:10:22Z

src/main/java/backend/model/payment/SponsoringInvoice.kt

@@ -118,7 +118,9 @@ class SponsoringInvoice : Invoice {
    }

    private fun Sponsoring.toEmailListing(): String {
-        return "<b>Team-ID</b> ${this.team?.id} <b>Teamname</b> ${this.team?.name} <b>Status</b> ${this.status} <b>Betrag pro km</b> ${this.amountPerKm.display()} <b>Limit</b> ${this.limit.display()} <b>Gereiste KM</b> ${this.team?.getCurrentDistance()} <b>Spendenversprechen</b> ${this.billableAmount().display()}"
+        // TODO: implement with teams


I'm honestly not sure how this should be correctly implemented.

We need to check who this is sent to? The sponsor? Each team?

Based on this we either list all teams here (if sent to sponsor) or add a team to the function parameters and only show this team's name+id+distance+km+billable amount. And not all other teams they're not interested in.

sibbl · 2022-05-16T10:11:11Z

src/main/java/backend/model/payment/SponsoringInvoiceRepository.kt

-        where s.id is not null or c.id is not null
+        left join i.sponsorings s
+        where c.id is not null
+        and exists (SELECT 1 FROM Sponsoring sp JOIN sp.teams t WHERE sp.id = s.id AND t.id = :teamId)


This SQL query is kind of untested. It might also be better to use and s.id IN (SELECT sp.id FROM Sponsoring sp JOIN sp.teams t WHERE sp.id = s.id AND t.id = :teamId) but my gut told me that exist might be faster.

sibbl · 2022-05-16T10:11:48Z

src/main/java/backend/model/sponsoring/Sponsoring.kt

-    @ManyToOne
-    var team: Team? = null
+    @ManyToMany
+    var teams: MutableSet<Team> = HashSet()


I chose a set for now. We might also want to use a list instead. I want to make sure that each team can only be once in there, so a Set felt better as a starting point.

sibbl · 2022-05-16T10:12:25Z

src/main/java/backend/model/sponsoring/Sponsoring.kt


    @ManyToOne(cascade = [PERSIST])
    private var unregisteredSponsor: UnregisteredSponsor? = null

    @ManyToOne
    private var registeredSponsor: Sponsor? = null

+    @ManyToOne
+    var event: Event? = null


This is new because the event was initially read from the single team's event field. However, we theoretically might have 0 teams and thus still need to store the event relation somewhere.

sibbl · 2022-05-16T10:13:34Z

src/main/java/backend/model/sponsoring/Sponsoring.kt

    @Suppress("UNUSED") //Used by Spring @PreAuthorize
    fun checkWithdrawPermissions(username: String): Boolean {
        return when {
-            this.unregisteredSponsor != null -> this.team!!.isMember(username)
+            this.unregisteredSponsor != null -> this.unregisteredSponsor!!.team?.isMember(username) == true


I'm a bit unsure whether this is correct. Can the one who created the sponsoring (so unregistedSponsor.team do this or any of the teams in the teams collection? Ideally it's the same team anyway.

We need to check this.

sibbl · 2022-05-16T10:14:00Z

src/main/java/backend/model/sponsoring/Sponsoring.kt

@@ -141,8 +153,9 @@ class Sponsoring : BasicEntity, Billable {
    }

    override fun billableAmount(): Money {
-
-        val distance: Double = team?.getCurrentDistance() ?: run {
+        val distance: Double = if (teams.isNotEmpty())


We need to implement the billing calculation across multiple teams here.

sibbl · 2022-05-16T10:14:39Z

src/main/java/backend/model/sponsoring/SponsoringService.kt

    fun rejectSponsoring(sponsoring: Sponsoring): Sponsoring

    @PreAuthorize("#sponsoring.checkWithdrawPermissions(authentication.name)")
    fun withdrawSponsoring(sponsoring: Sponsoring): Sponsoring

-    @PreAuthorize("#team.isMember(authentication.name)")
-    fun createSponsoringWithOfflineSponsor(team: Team, amountPerKm: Money, limit: Money, unregisteredSponsor: UnregisteredSponsor): Sponsoring
+    @PreAuthorize("#unregisteredSponsor.team.isMember(authentication.name)")


We need to check how this behaves in the real world. Perhaps we might need to check isTeamMember instead.

sibbl · 2022-05-16T10:15:02Z

src/main/java/backend/model/sponsoring/SponsoringServiceImpl.kt

@@ -154,7 +154,8 @@ class SponsoringServiceImpl(private val sponsoringRepository: SponsoringReposito
    }

    fun calculateAmount(sponsoring: Sponsoring): Money {
-        val kilometers = teamService.getDistanceForTeam(sponsoring.team!!.id!!)
+        // TODO: fix calculation
+        val kilometers = sponsoring.teams.sumByDouble { it.getCurrentDistance() }


Check if we need to calculate this on a per team base.

sibbl · 2022-05-16T10:15:24Z

src/main/java/backend/view/sponsoring/DetailedSponsoringView.kt

-    var teamId: Long? = null
-
-    var team: String? = null
+    var teams: Map<Long, String>? = null


This needs to be adapted in the frontend.

sibbl · 2022-05-16T10:15:28Z

src/main/java/backend/view/sponsoring/SponsoringView.kt

-    var teamId: Long? = null
-
-    var team: String? = null
+    var teams: Map<Long, String>? = null


This needs to be adapted in the frontend.

sibbl · 2022-05-16T10:16:01Z

src/main/java/backend/view/sponsoring/SponsoringView.kt

@@ -37,19 +35,19 @@ class SponsoringView() {

    var billableAmount: Double? = null

-    var teamDistance: Double? = null
+    var teamsTotalDistance: Double? = null


I'm unsure whether the total team distance is enough in here. If we need it on a per team base, we might need to extend the teams map to not only include the name as the value but also each team's individual distance.

Start implementing ManyToMany relation between Sponsoring and Team

191222a

sibbl commented May 16, 2022

View reviewed changes

Sebastian Schubotz added 3 commits May 16, 2022 12:50

Fix SQL migration

1c049fc

Renamve sponsoring_team to sponsoring_teams as required by hibernate

5d5a919

Fix hibernate startup issues

2e16713

sibbl mentioned this pull request May 20, 2022

Globale Challenges und Spenden als Sponsor eintragen können BreakOutEvent/breakout-frontend#401

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow one sponsoring to target multiple teams #264

Allow one sponsoring to target multiple teams #264

sibbl commented May 16, 2022

sibbl May 16, 2022

sibbl May 16, 2022

sibbl May 16, 2022

sibbl May 16, 2022 •

edited

Loading

sibbl May 16, 2022

sibbl May 16, 2022

sibbl May 16, 2022

sibbl May 16, 2022

sibbl May 16, 2022

sibbl May 16, 2022

sibbl May 16, 2022

sibbl May 16, 2022

sibbl May 16, 2022

Allow one sponsoring to target multiple teams #264

Are you sure you want to change the base?

Allow one sponsoring to target multiple teams #264

Conversation

sibbl commented May 16, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sibbl May 16, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sibbl May 16, 2022 •

edited

Loading