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

SAK-49341 Announcement NPE while viewing dashboard when announcements are merged from a site the user isn't a member of #11970

Merged
merged 4 commits into from
Oct 18, 2023

Conversation

austin48
Copy link
Contributor

@austin48 austin48 commented Oct 5, 2023

No description provided.

@@ -632,6 +633,8 @@ public Map<String, List<AnnouncementMessage>> getViewableAnnouncementsForCurrent

public List<AnnouncementMessage> getViewableAnnouncementsForSite(String channelRef, Integer maxAgeInDays) {

List<AnnouncementMessage> viewableAnnouncements = new Vector();
Copy link
Contributor

Choose a reason for hiding this comment

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

@austin48 a Vector probably isn't needed here unless it would be used by another thread?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Although the main case for this commit is returning an empty List where there is a failure getting messages, I used a Vector because in a successful case, getMessages() (from BaseAnnouncementService) returns a Vector.

Copy link
Contributor

Choose a reason for hiding this comment

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

Minor thing, but if the variable is not going to be used elsewhere I would directly return "new Vector" at the end of the method.

@ern
Copy link
Contributor

ern commented Oct 9, 2023

@austin48 this needs a jira issue

@austin48 austin48 changed the title NPE viewing dashboard containing announcements merged from site user isn't a member of SAK-49341 NPE viewing dashboard containing announcements merged from site user isn't a member of Oct 9, 2023
…site user isn't a member of

directly return empty vector at the end of the method
@ern ern changed the title SAK-49341 NPE viewing dashboard containing announcements merged from site user isn't a member of SAK-49341 Announcement NPE while viewing dashboard when announcements are merged from a site the user isn't a member of Oct 11, 2023
…announcement/impl/DbAnnouncementService.java
…announcement/impl/DbAnnouncementService.java
@ottenhoff ottenhoff merged commit 380c361 into sakaiproject:master Oct 18, 2023
2 of 3 checks passed
ern pushed a commit that referenced this pull request Oct 31, 2023
… are merged from a site the user isn't a member of (#11970)

Co-authored-by: Earle Nietzel <[email protected]>
(cherry picked from commit 380c361)
ern pushed a commit that referenced this pull request Oct 31, 2023
… are merged from a site the user isn't a member of (#11970)

Co-authored-by: Earle Nietzel <[email protected]>
(cherry picked from commit 380c361)
ern pushed a commit that referenced this pull request Oct 31, 2023
… are merged from a site the user isn't a member of (#11970)

Co-authored-by: Earle Nietzel <[email protected]>
(cherry picked from commit 380c361)
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.

4 participants