#6156: Footer image for Sub-Dataverse

[Gerafp]

New Contributors

Welcome! New contributors should at least glance at CONTRIBUTING.md, especially the section on pull requests where we encourage you to reach out to other developers before you start coding. Also, please note that we measure code coverage and prefer you write unit tests. Pull requests can still be reviewed without tests or completion of the checklist outlined below. Thanks!

Related Issues

• Closes Footer image and CSS customization for Sub-Dataverse #6156

Pull Request Checklist

• Unit tests completed
• Integration tests: None
• Deployment requirements, SQL updates, Solr updates, etc.: None
• Documentation completed
• Merged latest from "develop" branch and resolved conflicts

[dataversebot]

Can one of the admins verify this patch?

[coveralls]

Coverage decreased (-0.04%) to 19.449% when pulling 82dc1e7 on CIMMYT:6156-footer-image into ee874a6 on IQSS:develop.

[djbrooke]

Thanks @Gerafp for the PR. Assigning to myself to add some documentation for this.

Can you please put this on an AWS instance (like you did for #6154) so that we can review this more easily?

Thanks again!!

[djbrooke]

@Gerafp, I added the docs, please let us know when the branch is available and we'll review. Thanks!

[Gerafp]

Thanks @Gerafp for the PR. Assigning to myself to add some documentation for this.

Can you please put this on an AWS instance (like you did for #6154) so that we can review this more easily?

Thanks again!!

Hi @djbrooke

Yes, I deploy a instance here
http://ec2-52-203-33-8.compute-1.amazonaws.com:8080
dataverseAdmin
admin1

[djbrooke]

Thanks @Gerafp, we'll take a look tomorrow and provide feedback!

[djbrooke]

@TaniaSchlatter passing your way for a design review. I'm OK with how this implemented as it basically mirrors what we allow for the logo in the header, but feel free to provide design feedback to @Gerafp and @alejandratenorio if you have any.

@Gerafp, @alejandratenorio, we'll also start a review of the code itself. Thanks again for this contribution!

[djbrooke]

Thanks @TaniaSchlatter for discussing the feature and providing documentation feedback!

[pdurbin]

@djbrooke I approved this pull request but we could use this as an opportunity to refactor the code for both headers and footers and make it possible to set them via API. I believe @TaniaSchlatter recently asked if setting header logos for sample data is possible and it is not currently. Or we can, of course, defer adding these APIs (and API tests) until later.

[djbrooke]

Thanks for the review the and the note @pdurbin. Let's move this to QA and not expand it to the API since it's good to go and it meets the use case for the external contributor.

[kcondon]

@Gerafp When I deployed this it throws a 500 error, missing column, "logofooter", but I did not see a flyway entry to add this. Can you take a look?

[pdurbin]

@Gerafp hi! Two things:

• Here are the docs for our use of Flyway: http://guides.dataverse.org/en/4.16/developers/sql-upgrade-scripts.html . I'm happy to answer any questions here or http://chat.dataverse.org or https://groups.google.com/forum/#!forum/dataverse-dev . It was great talking to you and your team this morning about pull request #6155: OAuth 2.0 - Microsoft #6192!
• Since you're already spinning on Dataverse on EC2, I wanted to mention that I am working with @donsizemore on the ability to run our API test suite within EC2. You can track our progress at support API test suite dataverse-ansible#65 . I think I'm close. We recent removed a bunch of sleep statements from the API test suite and I'm hoping that adding a few back in will get the tests passing. Please stay tuned!

[djbrooke]

@Gerafp @pdurbin I made these updates because if we don't get this into 4.17 we'll need to update the filename again. :) Moving to QA.

[pdurbin]

@djbrooke sounds good. I changed the .3 to .1 in 1e9abbc

[pdurbin]

@kcondon @djbrooke it occurs to me that we should probably test with multiple Glassfish and update http://guides.dataverse.org/en/4.16/installation/advanced.html#multiple-glassfish-servers if necessary. Here's what's written there now with the important bit highlighted in blue:

[djbrooke]

Thanks @pdurbin for discussing. I:

• Updated the script name to 4.16.0.3.... because we plan to include it in 4.17
• I updated the documentation about Flyway to put more emphasis on the naming scheme using the previous version (since this confused me)
• Added the note about multiple glassfish servers that you recommended

[mheppler]

Found a few issues in the UI while demoing this feature on the test server that @kcondon set up. Created a new branch at 6156-dv-theme-footer-cleanup that includes my changes, which I've outlined below, as well as updates from the latest merges to the develop branch.

• dataverseFooterName, dataverseFooterTagline, dataverseFooterLink classes in CSS are not used in the XHTML and should be removed
• Fix spacing between new footer and copyright and Dataverse Project logo
• Footer overlaps custom footer
• Add "logo" to image alt text
• Remove Logo Format from footer because that is a header only feature

Will need to coordinate with @Gerafp to get this merged into his branch. (I tried to submit a PR against the original CIMMYT:6156-footer-image branch, but I couldn't seem to sort out how to do that in the GitHub.)