Skip to content
Snippets Groups Projects

Fix Error 500 when referencing issue with project in pending delete

Closed Fix Error 500 when referencing issue with project in pending delete
sh-fix-issue-31215 into master
2 unresolved threads
Closed Stan Hu requested to merge sh-fix-issue-31215 into master
2 unresolved threads

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
spec/lib/banzai/redactor_spec.rb
42 42 end
43 43 end
44 44
45 context 'when project is in pending delete' do
46 it 'redacts an issue attached' do
47 project.pending_delete = true
48 project.save
49 issue = create(:issue, project: project)
  • username-removed-423915
    username-removed-423915 @godfat ·
    Maintainer

    Does it make sense if we create the issue before marking the project as pending delete? Probably doesn't matter, but it would be more like real flow and in case we cannot associate a pending deleted project.

  • Please register or sign in to reply
  • username-removed-423915
    username-removed-423915 @godfat started a thread on the diff
    • spec/lib/banzai/redactor_spec.rb
    42 42 end
    43 43 end
    44 44
    45 context 'when project is in pending delete' do
    46 it 'redacts an issue attached' do
    47 project.pending_delete = true
    48 project.save
    49 issue = create(:issue, project: project)
    50 redactor = described_class.new(project, user)
    51 doc = Nokogiri::HTML.fragment("<a class='gfm' data-reference-type='issue' data-project=\"#{project.id}\" data-issue=\"#{issue.id}\">foo</a>")
    • username-removed-423915
      username-removed-423915 @godfat ·
      Maintainer

      I just tried to remove data-project and without the fix I could still reproduce the issue. I am not completely sure, but it looks like IssueParser overrides nodes_visible_to_user and data-project is not used here.

      I also found that there's also data-external-issue, which would use data-project. Perhaps we should also add a test for this?

    • username-removed-423915
      username-removed-423915 @godfat ·
      Maintainer

      I just wrote a test and data-external-issue is fine without the patch.

    • Please register or sign in to reply
  • username-removed-423915
    username-removed-423915 @godfat ·
    Maintainer

    @stanhu I left two comments. Do you want me to take it over and ask for a maintainer to review it?

  • username-removed-423915 mentioned in commit 31ea72b6

    mentioned in commit 31ea72b6

  • username-removed-423915 mentioned in merge request !10843 (merged)

    mentioned in merge request !10843 (merged)

  • username-removed-423915
    username-removed-423915 @godfat ·
    Maintainer

    @DouweM What do you think? I also made an MR based on this !10843 (merged) We probably also miss a changelog entry.

  • username-removed-423915 added regression label

    added regression label

  • username-removed-423915 assigned to @DouweM

    assigned to @DouweM

  • Stan Hu
    Stan Hu @stanhu ·
    Author Maintainer

    @godfat Please feel free to take over. Shall I close this MR in favor of !10843 (merged)?

    Edited by Stan Hu
  • Stan Hu closed

    closed

  • Stan Hu removed assignee

    removed assignee

  • Douwe Maan mentioned in commit d3a788da

    mentioned in commit d3a788da

  • Douwe Maan mentioned in commit 70fece35

    mentioned in commit 70fece35

    • Please register or sign in to reply