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

DS-9452: Updates query to get collections referenced in the subscribers table #9453

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

nwoodward
Copy link
Contributor

References

Description

This small PR updates the SQL query to get collections with subscribers.

Instructions for Reviewers

  1. Deploy this bug-fix branch
  2. Have at least one subscription to a collection in the subscriptions table
  3. Run [dspace]/bin/dspace healthcheck -e person@example.org
  4. Observe that the health check completes successfully now and lists subscriptions in the output

Checklist

  • My PR is small in size (e.g. less than 1,000 lines of code, not including comments & integration tests). Exceptions may be made if previously agreed upon.
  • My PR passes Checkstyle validation based on the Code Style Guide.
  • My PR includes Javadoc for all new (or modified) public methods and classes. It also includes Javadoc for large or complex private methods.
  • My PR passes all tests and includes new/updated Unit or Integration Tests based on the Code Testing Guide.
  • If my PR includes new libraries/dependencies (in any pom.xml), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.
  • If my PR modifies REST API endpoints, I've opened a separate REST Contract PR related to this change.
  • If my PR includes new configurations, I've provided basic technical documentation in the PR itself.
  • If my PR fixes an issue ticket, I've linked them together.

@tdonohue tdonohue added the 1 APPROVAL pull request only requires a single approval to merge. label Apr 10, 2024
@@ -159,7 +159,8 @@ public List<Collection> findAuthorizedByGroup(Context context, EPerson ePerson,

@Override
public List<Collection> findCollectionsWithSubscribers(Context context) throws SQLException {
return list(createQuery(context, "SELECT DISTINCT col FROM Subscription s join s.collection col"));
return list(createQuery(context, "SELECT c FROM Collection c JOIN Subscription s ON c.id = " +
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without using DISTINCT or GROUP BY there isn't any duplicated row (duplicated collection)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't notice duplicates in my testing, but you're correct that it's easy to prevent them. I added the DISTINCT function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 APPROVAL pull request only requires a single approval to merge. bug
Projects
Status: 🙋 Needs Reviewers Assigned
Development

Successfully merging this pull request may close these issues.

Health check fails because subscriptions table no longer references collections directly
3 participants