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 some issues with v2 #102

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Conversation

rafaucau
Copy link

@rafaucau rafaucau commented Nov 17, 2024

resolves #101

  • Improved performance by eliminating unnecessary database queries
  • Fixed error when discussion author has deleted account

rafaucau and others added 2 commits November 17, 2024 13:11
@rafaucau
Copy link
Author

@jaspervriends what is minimum supported PHP version by this extension?

@rafaucau rafaucau marked this pull request as ready for review November 17, 2024 14:41
@rafaucau
Copy link
Author

rafaucau commented Nov 17, 2024

This causes a huge performance issue when the discussion contains thousands of posts:

$posts = $discussion->posts()
->where('number', '>', '1')->get();

What would be the preferred approach for handling this - limiting the number of loaded posts or skipping this functionality entirely for discussions with a high post count? I can submit a separate PR once we determine the best solution. Meanwhile, this PR can be merged as it improves other aspects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invalid code in DiscussionBestAnswerPage, condition always true
1 participant