853979 - "submitted at" field doesn't always display on ship it's completed releases (and maybe elsewhere)

Status: RESOLVED FIXED

(Release Engineering :: Release Automation: Other, defect)

Description

[[e-mail] (:bhearsum)]

Massimo, you pointed this out to me in dev, but I thought it was just because of the data in dev. Turns out this is an issue in production too...so there's something mucked up somewhere.

Comment 1

[Massimo Gervasini [:massimo]]

Attachment: added a function to update submitted_at when table changes

I think the issue is caused by some lazy load mechanism in dataTables library. Elements on the first page are fine, rows in the next pages are empty or show the wrong data. 

This patch adds a toLocalDate() function that is called when the page is loaded and when the table changes. 

I've tested this patch locally and it works fine. Is it possible to have a production database dump to check how it behaves with real data?

Comment 2

[[e-mail] (:bhearsum)]

Comment on attachment 728977 [details] [diff] [review]
added a function to update submitted_at when table changes

Review of attachment 728977 [details] [diff] [review]:
-----------------------------------------------------------------

This patch doesn't work for me...I end up with "invalid date" for all of the releases. I tried to debug a bit locally and it looks toLocaleDate is called once at page load, and then once right afterwards (probably on the fwDrawCallback event). I think what's happening is it's getting set correctly on the first call, and then it tries to read in the already localized date on the second call and doesn't know what to do with it.

::: kickoff/templates/includes/releases_reviewed.html
@@ +64,2 @@
>  <script type='text/javascript'>
> +function toLocalDate() {

This is getting defined twice...can you put it in a central place? Maybe it's time to have a kickoff.js for things like this?

Comment 3

[[e-mail] (:bhearsum)]

Comment on attachment 728977 [details] [diff] [review]
added a function to update submitted_at when table changes

Review of attachment 728977 [details] [diff] [review]:
-----------------------------------------------------------------

We did a bit more debugging on this. My hunch about things being called multiple times was right -- toLocalDate gets called twice on load (once in releases_complete, once in releases_reviewed). The second time it gets called, its input is an already localized date. Depending on your system language format, this may or may not work (it worked for Massimo, didn't work for me). I don't think fnDrawCallback is necessary here - in my local fiddling it only gets called once on page load, which makes it effectively the same as running something through $(document).ready(). I think the only thing needed is a _single_ call to toLocalDate(). This should probably be '$(document).ready(toLocalDate)' in releases.html.

::: kickoff/templates/submit_release.html
@@ -24,5 @@
>     {% endfor %}
>  </div>
> -<script>
> - $(document).ready(function() {
> -   $( "#submitrelease" ).prop('checked', true);

Also, what's this change about? It seems unrelated.

Comment 4

[Massimo Gervasini [:massimo]]

Attachment: fix for empty submitted_at columns

I had problems calling the local date function on document ready so I had to split the toLocalDate in two different functions: one for complete/reviewed releases and one for submitted releases. 

I've also moved javascript code into kickoff.js as suggested in comment #2

Comment 5

[[e-mail] (:bhearsum)]

Comment on attachment 732795 [details] [diff] [review]
fix for empty submitted_at columns

Review of attachment 732795 [details] [diff] [review]:
-----------------------------------------------------------------

::: kickoff/static/kickoff.js
@@ +1,3 @@
> +function initialSetup(){
> +  $( "#tabs" ).tabs();
> +  $( "#accordion" ).accordion({ heightStyle: "content" });

I see aaSorting disappearing...is that intended? Do you need to merge/rebase?

@@ +5,5 @@
> +
> +function onTableChange(release_type) {
> +    // entry point for dataTable events
> +    toLocalDate(release_type);
> +}

What's the point of this function? Can't you use toLocalDate as the callback directly?

::: kickoff/templates/releases.html
@@ +31,5 @@
> +<script>
> +    $(document).ready(function(){
> +        $( "#reviewed" ).dataTable({
> +            "bJQueryUI": true,
> +            "aaSorting": [[ 2, "desc" ]], // sort by SubittedAt

typo nit: SubittedAt vs. SubmittedAt.

@@ +32,5 @@
> +    $(document).ready(function(){
> +        $( "#reviewed" ).dataTable({
> +            "bJQueryUI": true,
> +            "aaSorting": [[ 2, "desc" ]], // sort by SubittedAt
> +            "fnDrawCallback": onTableChange('reviewed'),

Are you sure this works? It looks like you're using the return value of onTableChange() as the callback. I think this is effectively the same as calling onTableChange in document.ready()....

If you're calling this here you need to wrap it in a function(){}.

@@ +39,5 @@
> +            "bJQueryUI": true,
> +            "aaSorting": [[ 2, "desc" ]], // sort by SubittedAt
> +            "fnDrawCallback": onTableChange('complete'),
> +        });
> +    updateSubmitLocalDate();

You're effectively doing the same thing twice again....

I tried this locally:
<script>
    $(document).ready(function(){
        $( "#reviewed" ).dataTable({
            "bJQueryUI": true,
            "aaSorting": [[ 2, "desc" ]], // sort by SubittedAt
            "fnDrawCallback": toLocalDate('reviewed'),
        });
        $( "#complete" ).dataTable({
            "bJQueryUI": true,
            "aaSorting": [[ 2, "desc" ]], // sort by SubittedAt
            "fnDrawCallback": toLocalDate('complete'),
        });
});


and deleted the onTableChange and updateSubmitLocalDate functions, and everything still worked fine. I think this code is still overcomplicating the issue.

Comment 6

[Massimo Gervasini [:massimo]]

Attachment: fix for empty submitted_at columns

Hi Ben,

I've done another test running toLocalDate() when the page is ready. It works fine only if the tables have a single page, from the second page, dates are just the same string provided by the backend.

The attached patch resolves the issue updating 'submitted at' fields before initializing the datatables, so when table is loaded, dates are already in the correct format and there is no need for fnDrawCallback.

Comment 7

[[e-mail] (:bhearsum)]

I pushed this to ship it prod.