Zhen Qian

  • More
  • CR-31
  • finished reviewing
Looks good to me.

Looks good to me.

What if the name, url, or type of the attachment is null? Will that cause NPE?

What if the name, url, or type of the attachment is null? Will that cause NPE?

Yes. After this change, the "url" field of content entity feed will be the original URL for Web Link resources. For other types of resource items, the "URL" attribute will be the normal access url.

Yes. After this change, the "url" field of content entity feed will be the original URL for Web Link resources. For other types of resource items, the "URL" attribute will be the normal access url.

truncated URL String returned in content entity feed for Web Link resources
truncated URL String returned in content entity feed for Web Link resources
checks for resource copyright alert setting
checks for resource copyright alert setting
  • More
  • CR-28
  • finished reviewing
Looks good to me! The same SQL has been applied to ctqa DB instance successfully.

Looks good to me! The same SQL has been applied to ctqa DB instance successfully.

Done. Merged into CTools branch. Created PR for Sakai: https://github.com/sakaiproject/sakai/pull/1731

Done.

Merged into CTools branch.

Created PR for Sakai: https://github.com/sakaiproject/sakai/pull/1731

Done. Refactored into getSiteMailChannels(String siteId)

Done. Refactored into getSiteMailChannels(String siteId)

Done. Refactored into getSiteMailChannels(String siteId)

Done. Refactored into getSiteMailChannels(String siteId)

replaced with mail.read

replaced with mail.read

addressed.

addressed.

SAK-30211 add MailArchive EntityProvider
SAK-30211 add MailArchive EntityProvider
same as above. Comments needed.

same as above. Comments needed.

could be "return !ac.isEmpty();"

could be "return !ac.isEmpty();"

please put some comments for the newly added functions.

please put some comments for the newly added functions.

  • More
  • CR-25
  • finished reviewing
Changes look good!

Changes look good!

Same comment as Beth's. I do not see the feed value used.

Same comment as Beth's. I do not see the feed value used.

Should the item.getItemCount() ==1? What does "item.getItemCount() > 1" mean?

Should the item.getItemCount() ==1? What does "item.getItemCount() > 1" mean?

Log error/warning when the source type is null

Log error/warning when the source type is null

Yes. https://crucible.sakaiproject.org/cru/CONTRIB-79 It has been reviewed and closed

Yes. https://crucible.sakaiproject.org/cru/CONTRIB-79

It has been reviewed and closed