Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix JS NPE when viewing specific range of PR commits #27912

Merged
merged 4 commits into from
Nov 6, 2023

Conversation

delvh
Copy link
Member

@delvh delvh commented Nov 5, 2023

This should be the easiest fix.
While other solutions might be possible that exterminate the root cause, they will not be as trivial.

This is the easiest fix.
While other solutions might be possible, they will not be as trivial.
@delvh delvh added type/bug topic/ui-interaction Change the process how users use Gitea instead of the visual appearance backport/v1.21 This PR should be backported to Gitea 1.21 labels Nov 5, 2023
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Nov 5, 2023
@pull-request-size pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Nov 5, 2023
@silverwind
Copy link
Member

NPE surely isn't the right term? JavaScript does not have pointers 😆

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Nov 5, 2023
@delvh
Copy link
Member Author

delvh commented Nov 5, 2023

Technically, JS calls it TypeError.
But still, it is basically a NullPointerException, simply as that's the term that everyone knows.
Java has no explicit pointers either, and still it's a NullPointerException there… 😁

@delvh
Copy link
Member Author

delvh commented Nov 5, 2023

Update: Okay, I just found another bug in the same functionality:
Whenever I select a single commit in that menu, it shows showing only changes of commit <non-existing commit SHA>.
The URL is correct, however.

@delvh
Copy link
Member Author

delvh commented Nov 5, 2023

@sebastian-sauer do you have any idea perhaps what might be causing ?

@sebastian-sauer
Copy link
Contributor

I'll try to have a look into that.

@sebastian-sauer
Copy link
Contributor

sebastian-sauer commented Nov 5, 2023

Looks like the reference to BeforeCommitID is wrong in the tmpl https://github.com/go-gitea/gitea/blob/main/templates/repo/diff/box.tmpl#L56

When only one commit is shown this should use AfterCommitID

PR: #27916

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Nov 5, 2023
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Nov 6, 2023
@lunny lunny enabled auto-merge (squash) November 6, 2023 01:53
@lunny lunny merged commit 1f501da into go-gitea:main Nov 6, 2023
25 checks passed
@GiteaBot GiteaBot added this to the 1.22.0 milestone Nov 6, 2023
GiteaBot pushed a commit to GiteaBot/gitea that referenced this pull request Nov 6, 2023
This should be the easiest fix.
While other solutions might be possible that exterminate the root cause,
they will not be as trivial.
@GiteaBot GiteaBot added the backport/done All backports for this PR have been created label Nov 6, 2023
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Nov 6, 2023
lunny pushed a commit that referenced this pull request Nov 6, 2023
Backport #27912 by @delvh

This should be the easiest fix.
While other solutions might be possible that exterminate the root cause,
they will not be as trivial.

Co-authored-by: delvh <dev.lh@web.de>
zjjhot added a commit to zjjhot/gitea that referenced this pull request Nov 6, 2023
* upstream/main:
  Fix edit topic UI (go-gitea#27925)
  Unify two factor check (go-gitea#27915)
  Revert go-gitea#27870 (go-gitea#27917)
  Fix JS NPE when viewing specific range of PR commits (go-gitea#27912)
  Install poetry dependencies with --no-root (go-gitea#27919)
  Show correct commit sha when viewing single commit diff (go-gitea#27916)
  Fix 500 when deleting a dismissed review (go-gitea#27903)
  Remove action runners on user deletion (go-gitea#27902)
  Remove SSH workaround (go-gitea#27893)
  Remove "tabindex" from some form buttons (go-gitea#27892)
  Refactor the function RemoveOrgUser (go-gitea#27582)
  Fix DownloadFunc when migrating releases (go-gitea#27887)
@delvh delvh deleted the bugfix/no-js-npe branch November 13, 2023 18:06
fuxiaohei pushed a commit to fuxiaohei/gitea that referenced this pull request Jan 17, 2024
This should be the easiest fix.
While other solutions might be possible that exterminate the root cause,
they will not be as trivial.
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Feb 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created backport/v1.21 This PR should be backported to Gitea 1.21 lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. topic/ui-interaction Change the process how users use Gitea instead of the visual appearance type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants