Uploaded image for project: 'VIVO'
  1. VIVO
  2. VIVO-185

Move visualization URL building out of templates

    XMLWordPrintable

    Details

    • Attachments:
      0
    • Comments:
      0

      Description

      Moved from https://issues.library.cornell.edu/browse/NIHVIVO-2015, originally created by Rebecca Younes and with a long thread of comments:

      Nick, I've been thinking more about the case of the visualization url and related examples. I don't want to rock the boat for 1.2, so we'll leave everything as is, but I would like to take url-building out of the templates for 1.3. Here's my thinking on why this belongs in the controllers: ideally information should be stored in only one layer of the app, to reduce coupling among the layers. The application layer is responsible for routing, so it has to know how to create urls to various destinations. This knowledge should not also reside in the templates, so when the templates need to display a link to some destination, that destination should be provided by the controllers. This was my original design and why I initially didn't even give the templates the ability to create urls. As template authors started trying to work around that constraint by using ${urls.home}, which wasn't going to produce the right results in all cases, I relented and provided ${urls.base} (i.e., the context path).

      I would rather have the visualization url back in the IndividualTemplateModel than have the template produce it, even if that means putting back that vivo-specific method into vitro. We're now tempted to shove vivo-specific logic into the templates even if it doesn't belong there, simply because you have a well-structured way of handling the vivo-vitro distinction in the templates. There is a good way of handling this through inheritance in the template models, which I simply haven't implemented but could quite easily.

      Note to myself: put all current IndividualTemplateModel methods into abstract BaseIndividualTemplateModel. Vitro IndividualTemplateModel contains no additional methods. Vivo IndividualTemplateModel contains vivo-specific methods.

      Details
      Type:
      Sub-task
      Status:
      Open
      Priority: Blocker
      Resolution:
      Unresolved
      Affects Version/s: None
      Fix Version/s:
      v1.6
      Component/s:
      Visualization improvements
      Labels:
      None
      Team:
      Application Team, UI Team
      Description
      Nick, I've been thinking more about the case of the visualization url and related examples. I don't want to rock the boat for 1.2, so we'll leave everything as is, but I would like to take url-building out of the templates for 1.3. Here's my thinking on why this belongs in the controllers: ideally information should be stored in only one layer of the app, to reduce coupling among the layers. The application layer is responsible for routing, so it has to know how to create urls to various destinations. This knowledge should not also reside in the templates, so when the templates need to display a link to some destination, that destination should be provided by the controllers. This was my original design and why I initially didn't even give the templates the ability to create urls. As template authors started trying to work around that constraint by using ${urls.home}, which wasn't going to produce the right results in all cases, I relented and provided ${urls.base} (i.e., the context path).

      I would rather have the visualization url back in the IndividualTemplateModel than have the template produce it, even if that means putting back that vivo-specific method into vitro. We're now tempted to shove vivo-specific logic into the templates even if it doesn't belong there, simply because you have a well-structured way of handling the vivo-vitro distinction in the templates. There is a good way of handling this through inheritance in the template models, which I simply haven't implemented but could quite easily.

      Note to myself: put all current IndividualTemplateModel methods into abstract BaseIndividualTemplateModel. Vitro IndividualTemplateModel contains no additional methods. Vivo IndividualTemplateModel contains vivo-specific methods.

      Nick Cappadona added a comment - 03/Feb/11 10:25 AM
      Rebecca, I agree that for something like the visualization url it makes sense for it to be delivered to the template via the controller and in general I agree with reducing the coupling among layers. My only hope is that this isn't taken to an extreme and the following is deemed inappropriate for templates:

      <link rel="stylesheet" href="${urls.base}/css/vitro.css" />
      <script type="text/javascript" src="${urls.base}/js/vitroUtils.js"></script>
      <link rel="stylesheet" href="${urls.theme}/css/screen.css" />

      Rebecca Younes (account no longer used) added a comment - 03/Feb/11 10:33 AM
      No, absolutely not. These are the cases where it is legitimate for the templates to create urls - since these locations are relevant only to the templates, and not to the controllers. That being said, if the locations needed for css, js, and image files are consistent enough so that they can be provided with a few stable variables like ${urls.theme}, we could remove ${urls.base} to prevent the kind of creep we've been seeing. But I don't want to create a situation where the UI team has to make a request to the app team every time it decides to put a jss, cs, or image file in a new location, so if those locations aren't enumerable in advance, we'll have to provide for that.

      Nick Cappadona added a comment - 03/Feb/11 10:51 AM
      Yeah, I guess that's my fear. That we would attempt to replace $[urls.base} with a proliferation of variables that are essentially ${urls.base} + 'insertDirectoryStructure' here. Which, like you said, would result in anyone working at the template level having to go to FreemarkerHttpServlet and add a new variable to the urls hash.

      And I think it's important to understand that this is not just a UI team vs. application team interaction here. In other words, that's just one aspect of this conversation, the part about how the development team handles this trade-off. But the intent is for this app to be used by people outside of the development team. You can download it and create your own theme or use an existing theme that the community provides. And the power of the themes truly lies in the templating system on which they are built. So yes, the de-coupling is important, but at the same time it's important that we do not handcuff our users who are outside development. They don't want to mess with the controllers. They just want to write templates so they can build and present the app as they please. So as far as I'm concerned those locations are not enumerable in advance as I don't think we should begin to attempt to make assumptions that would meet everyone's needs.
      Rebecca Younes (account no longer used) added a comment - 03/Feb/11 11:44 AM
      Your points are well-taken. So we'll plan to keep ${urls.base}, and to move the visualization url and any other urls that derive from the application structure to the controllers. Sound good?
      Nick Cappadona added a comment - 03/Feb/11 12:31 PM
      Sounds wonderful. Thanks
      Rebecca Younes (account no longer used) added a comment - 23/Mar/11 1:00 PM
      Much of the remaining work is in the visualization code, and I will need to either pass that on to Chintan or discuss it with him.
      Rebecca Younes (account no longer used) added a comment - 22/Jun/11 11:37 AM
      Moving to 1.4.
      Rebecca Younes (account no longer used) added a comment - 08/Aug/11 5:28 PM
      Assigning to Nick so he can work with Chintan on this, since I will not have time for it. As noted in an earlier comment, the remaining urls are primarily in the visualization templates. There may be others here and there. Nick, I think you're familiar with my intentions in this area.
      Chintan Tank added a comment - 08/Aug/11 5:38 PM
      Nick might need to work with Chin on this, since from now on I am in "only documentation" mode (workshop conference + other stuff). Adding Chin as the watcher. Although even he might end up delegating this to the new "me" here.

      But in any case I would like to know about an example of visualization template which is violating this jira issue's intent, so that I can pass on any relevant knowledge or documentation to Chin.
      Chintan Tank added a comment - 08/Aug/11 5:39 PM
      The issue has just been emailed to: "kongch@indiana.edu", with subject: "Move url building out of templates", and content:
      Jon Corson-Rikert added a comment - 07/Oct/11 4:14 PM
      We will have to discuss this with Chin Hua in looking at priorities for v1.5
      Chin Hua Kong added a comment - 14/Nov/11 11:49 AM
      Nick and Rebecca, please update the status of this issue. What are the remaining tasks? What should I work on?
      Chin Hua Kong added a comment - 27/Feb/12 11:59 AM
      Hi Tim, do you have any idea on this issue?
      Tim Worrall added a comment - 27/Feb/12 3:07 PM
      Chin Hua: I've added Brian Caruso as a watcher to this issue because this is more of an application issue than a UI or usability issue. I'm not familiar at all with the IndividualTemplateModel, but that's a class that falls into Brian's area. So he may be able to help here.

        Attachments

          Activity

            People

            • Assignee:
              j2blake Jim Blake
              Reporter:
              jc55 Jon Corson-Rikert
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:

                Time Tracking

                Estimated:
                Original Estimate - 2 hours
                2h
                Remaining:
                Remaining Estimate - 2 hours
                2h
                Logged:
                Time Spent - Not Specified
                Not Specified