LPS-76354 #391
LPS-76354 #391
Conversation
…cutions of same test class https://issues.liferay.com/browse/LPS-76324
…s behavior in the back end instead of relying on a disabled checkbox
|
Comment ci:test to trigger CI tests. |
|
ci:test |
|
Pull request test invoked at http://test-1-17.liferay.com/job/test-portal-acceptance-pullrequest(master). |
|
Thanks for sending this my way, @jonathanmccann. I'll try to take a look today. |
|
Hey @jonathanmccann what is the thought behind updating the AnnouncementsDelivery and User apis instead of just hard-coding true into the AnnouncementsDelivery in the ActionCommand? Seems like that would accomplish the same thing. Is it to prevent that value from being updated in the same way anywhere else? I'm not too familiar with the purpose of AnnouncementsDelivery, but this has me wondering what that field is even for. |
|
An additional change we can consider making is to remove the checkbox from the UI, but I'm not sure if that violates some hidden product definition ;) |
|
I wanted to make sure that the value wouldn't be updated to false via any other means. The field itself is completely useless. It's never read anywhere and is only there for visual purposes. This is why I wanted to remove it completely, but that caused a major version increase in portal-kernel, like I mentioned. Also, like you, I'm not sure if someone somewhere enjoys having those checkboxes shown. |
|
@jonathanmccann that makes sense, thanks for clarifying. In that case I'm fine with these changes. |
|
Pull request forwarded to brianchandotcom#53599. See changes here. |
Sent from 
@drewbrokke Sending this through you since it seems more related to Users than Announcements, but let me know if you'd like to have this go elsewhere.
Based off of LPS-216, it seems the Website option should always be checked, however, we're depending on disabled checkboxes to achieve that. Instead, we should enforce this rule in the backend so that it never changes.
I would have liked to remove the entire Website option since I don't see the use for it but that requires a major version increase to portal-kernel which I'd like to avoid.
Let me know if you have any questions.