Contribution defn diffs #15
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ChrisPenner
commented
Jul 29, 2024
Comment on lines
-17
to
-18
| loadNamespaceIdForCausal, | ||
| expectNamespaceIdForCausal, |
There was a problem hiding this comment.
These don't fit the new traversal loader pattern, and already had a replacement, so I replaced all usages of these.
ChrisPenner
commented
Jul 29, 2024
| @@ -235,9 +240,9 @@ contributionDiffEndpoint (AuthN.MaybeAuthedUserID mayCallerUserId) userHandle pr | |||
| newPBSH <- Codebase.runCodebaseTransactionOrRespondError newCodebase $ do | |||
| lift $ Q.projectBranchShortHandByBranchId newBranchId `whenNothingM` throwError (EntityMissing (ErrorID "branch:missing") "Source branch not found") | |||
|
|
|||
| let cacheKeys = [IDs.toText contributionId, IDs.toText newPBSH, IDs.toText oldPBSH, Caching.causalIdCacheKey newBranchCausalId, Caching.causalIdCacheKey oldBranchCausalId] | |||
| let oldCausalId = fromMaybe oldBranchCausalId bestCommonAncestorCausalId | |||
| let cacheKeys = [IDs.toText contributionId, IDs.toText newPBSH, IDs.toText oldPBSH, Caching.causalIdCacheKey newBranchCausalId, Caching.causalIdCacheKey oldCausalId] | |||
There was a problem hiding this comment.
I altered the caching key to use the BCA so the cache will blow less often (but will still be correct)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
A small wrapper around the existing term and type diff apis which is scoped to contributions. This will allow us to use the best common ancestor for definition diffs, solving the bug we have where the diff of a PR changes unexpected when it's merged.
Implementation
Adds two new endpoints:
/users/<handle>/projects/<slug>/contributions/<n>/diff/terms?oldTerm=old.name&newTerm=new.name/users/<handle>/projects/<slug>/contributions/<n>/diff/terms?oldType=old.name&newType=new.nameThese behave identically to the existing definition diff endpoints except that they use the best-common-ancestor that's saved on the contribution as the diff base.