Implement match and match cycle repos

[rootandroo]

No description provided.

[rootandroo]

• Implement match and match cycle repos #74 : 2 dependent PRs (#80 , #86 ) 👈 (View in Graphite)
• Modify match and match cycle columns  #79
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[graphite-app]

Graphite Automations

"Request reviewers once CI passes" took an action on this PR • (08/10/26)

2 reviewers were added to this PR based on Henry Chen's automation.

[spiffyy99]

Code looks good, although I wonder if the match cycle repo could use some per-field update methods like match has.

Also, we planning on writing any tests for these? Not sure if there's another ticket for them

[rootandroo]

Code looks good, although I wonder if the match cycle repo could use some per-field update methods like match has.

Also, we planning on writing any tests for these? Not sure if there's another ticket for them

​thanks for the feedback il include tests in the next PR. Will update this one with the per field methods

[arklian]

One other thing is that you're building out everything together layer by layer, which makes sense with how we planned it out and with AI implementing it, but it would be easier to build it out feature by feature once we get it to a little more stable state.

Like having the basics set, and then adding filtering and feedback later down, instead of including it from the start.

[sonarqubecloud]

❌ The last analysis has failed.

See analysis details on SonarQube Cloud

[spiffyy99]

Nit: better titled as setMatchCycleIsDraft, this way it's clearer that it's a Boolean switch.

[spiffyy99]

Postgres has range types we can use to do range searches, that would probably be a better type fit for this field. This would probably be more effective than shoehorning the range into a single format. Unsure how much we'll need that in the future though.

[spiffyy99]

A few thoughts here:

• would we benefit from including member emails so the queries would be simpler? I'm assuming member ids are not indexed right now so it probably wouldn't make a difference performance wise. We could also include both in the table
• We should have some kind of unique constraint or at least index on the pairing of member an and b's ids/emails. So we can easily look up "have a and b matched before? Or have b and a matched before"? Because that would be an instantly disqualifying factor.

[graphite-app]

Critical Bug: Missing parameter binding for total_members

The INSERT statement on line 37 includes "total_members" as a column and references :total_members as a parameter on line 43, but this parameter is never bound in the JDBC client call (lines 50-55).

The MatchCycle model also doesn't have a totalMembers field, and based on the interface documentation changes (lines 15-20 in MatchCycleRepo.java), this field was intentionally removed.

Impact: This will cause a SQL parameter binding error at runtime when attempting to create a match cycle.

Fix: Remove the total_members column and parameter from the INSERT statement:

String sql = """
    INSERT INTO "match_cycles" (
        "period",
        "run_at",
        "is_draft"
    )
    VALUES(
        :period,
        :run_at,
        :is_draft
    )
    RETURNING
        *,
        %s
""".formatted(TOTAL_MATCHED_SQL);

Suggested change

	String sql = """
	INSERT INTO "match_cycles" (
	"period",
	"run_at",
	"is_draft",
	"total_members"
	)
	VALUES(
	:period,
	:run_at,
	:is_draft,
	:total_members
	)
	RETURNING
	*,
	%s
	""".formatted(TOTAL_MATCHED_SQL);
	return jdbc.sql(sql)
	.param("period", matchCycle.getPeriod())
	.param("run_at", matchCycle.getRunAt())
	.param("is_draft", matchCycle.isDraft())
	.query((rs, rowNum) -> parseResultSetToMatchCycle(rs))
	.single();
	}
	String sql = """
	INSERT INTO "match_cycles" (
	"period",
	"run_at",
	"is_draft"
	)
	VALUES(
	:period,
	:run_at,
	:is_draft
	)
	RETURNING
	*,
	%s
	""".formatted(TOTAL_MATCHED_SQL);
	return jdbc.sql(sql)
	.param("period", matchCycle.getPeriod())
	.param("run_at", matchCycle.getRunAt())
	.param("is_draft", matchCycle.isDraft())
	.query((rs, rowNum) -> parseResultSetToMatchCycle(rs))
	.single();
	}

Spotted by Graphite



Is this helpful? React 👍 or 👎 to let us know.

[sonarqubecloud]

Quality Gate passed

Issues
8 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[graphite-app]

The code calls rs.getObject("match_score", Float.class) twice - once for the null check and once for the conversion. This is inefficient and could cause issues with certain JDBC drivers. Store the result in a variable first.

Float scoreFloat = rs.getObject("match_score", Float.class);
.matchScore(scoreFloat == null ? null : scoreFloat.doubleValue())

Spotted by Graphite



Is this helpful? React 👍 or 👎 to let us know.