[GitHub] [airavata-django-portal] machristie commented on a diff in pull request #88: AIRAVATA-3574, AIRAVATA-3575: Enabling workspace documentation (notification based) and application instructions

[GitBox <gi...@apache.org>]

machristie commented on code in PR #88:
URL: https://github.com/apache/airavata-django-portal/pull/88#discussion_r886163987


##########
django_airavata/apps/api/serializers.py:
##########
@@ -1081,11 +1081,39 @@ class NotificationSerializer(
     publishedTime = UTCPosixTimestampDateTimeField()
     expirationTime = UTCPosixTimestampDateTimeField()
     userHasWriteAccess = serializers.SerializerMethodField()
+    showInDashboard = serializers.BooleanField(default=False)
 
     def get_userHasWriteAccess(self, userProfile):
         request = self.context['request']
         return request.is_gateway_admin
 
+    def validate(self, attrs):
+        del attrs["showInDashboard"]
+
+        return attrs
+
+    def to_representation(self, notification):
+        notification_extension_list = models.NotificationExtension.objects.filter(
+            notification_id=notification.notificationId)
+        setattr(notification, "showInDashboard",
+                False if len(notification_extension_list) == 0 else notification_extension_list[0].showInDashboard)
+
+        return super().to_representation(notification)
+
+    def update_notification_extension(self, request, notification):

Review Comment:
   Please override the `create` and `update` methods to manage the showInDashboard value. As an example, see how showQueueSettings is a recent commit: https://github.com/apache/airavata-django-portal/commit/2ba4bccc665dbc83ca5b4c15a1a27c072fe37487#diff-6375089af70efb2f3674dcd2b4e71b5d218748ac7d276e38486fd0e0370d22d5



##########
django_airavata/apps/workspace/static/django_airavata_workspace/js/components/notices/WorkspaceNoticesManagementContainer.vue:
##########
@@ -0,0 +1,49 @@
+<template>
+  <div class="w-100">
+    <ul style="list-style: none; margin: 0px; padding: 0px;">
+      <li v-for="(notice, noticeIndex) in notices" :key="noticeIndex">
+        <b-alert show>
+          <div class="d-flex flex-row">
+            <strong class="flex-fill" style="white-space: pre;">{{ notice.title }}</strong>
+            <human-date v-if="notice.publishedTime" :date="notice.publishedTime" style="font-size: 10px;"/>
+          </div>
+          <div style="white-space: pre;font-size: 12px;">{{ notice.notificationMessage }}</div>

Review Comment:
   Can you wrap the notificationMessage in `<linkify>`?  Like this for example: https://github.com/apache/airavata-django-portal/blob/develop/django_airavata/static/common/js/components/ApplicationCard.vue#L23-L25



##########
django_airavata/apps/workspace/static/django_airavata_workspace/js/containers/CreateExperimentContainer.vue:
##########
@@ -53,6 +53,7 @@ export default {
     Promise.all([loadAppModule, loadAppInterface])
       .then(([appModule, appInterface]) => {
         const experiment = appInterface.createExperiment();
+        experiment.appInterface = appInterface;

Review Comment:
   `appInterface` isn't a property of the experiment model. I would suggest that you add this to data object, similar to `appModule`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@airavata.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org