Shopping cart bug tracker

[Gibbsdavidl]

General issue to track bugs and issues related to the shopping cart.

[Gibbsdavidl]

Cart bug 1

1. Under FDA, select VICTRE, open chevron in Collections to open Cases
2. Click on case level cart … nothing happens
3. Click again… now it's selected.

Expectation is that it would be selected the first time it's clicked.

[Gibbsdavidl]

Cart observation 1

After opening cases and making independent selections across pages, I was surprised to find that when a new case chevron was opened, the past chevrons were not closed. This leads to a collection of studies.

My intuition was that each time a case chevron was opened, what was displayed in the studies was only the last case that was opened.

Here's an image after making a number of selections across pages:

[Gibbsdavidl]

Cart observation 2 (or bug?)

1. make some cart selections
2. open the cart screen
3. then click on 'explore images' button in the top menu
4. the cart is now empty

Expectation is that there would be no changes to the cart.

[fedorov]

@G-White-ISB the explore page looks really nice! I would actually love for that UI facelift to just go to the main portal without waiting for the cart to be finished (but definitely let's not make anyone's life even more complex by separating those, unless its not difficult).

I spent some time with this today, and here are some findings. Some of those are unfortunately seem to be rather serious.

• For the tooltip, I think it might be more accurate to rephrase it to say "X series selected from K collections/L cases/M studies". The way it is phrased right now seems to suggest that entire collections/cases/studies were selected. Also, I would consider showing cart summary not in the tooltip, but next to the button. We do not have any use to that whitespace, and we do show content of a cohort when selected using the filter in the panel - not in the tooltip, so this would be more consistent.

• Can we add size of the cart selection in TB? This would be useful, and consistent with the cohort selection.
• trying to download the manifest for the the cart shown above gave me "Internal Server Error - 500: There was an error while handling your request." Console did have more details:

 Error: Mismatched anonymous define() module: function($, utils) {


    // Resets forms in modals on hide. Suppressed warning when leaving page with dirty forms
    $('.modal').on('hide.bs.modal', function () {
        if(!$(this).prop("saving")) {
            if($(this).find('form').get().length) {
                $(this).find('form').get(0).reset();
            }
        }
    });

    $.getCookie = utils.getCookie;
    $.setCookie = utils.setCookie;
    $.removeCookie = utils.removeCookie;

    return {
        blacklist: /<script>|<\/script>|!\[\]|!!\[\]|\[\]\[\".*\"\]|<iframe>|<\/iframe>/ig,
        // From http://www.regular-expressions.info/email.html
        email: /^[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*@(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?$/,
        showJsMessage: utils.showJsMessage,
        // Simple method for standardizing storage of a message into sessionStorage so it can be retrieved and reloaded
        // at document load time
        setReloadMsg: function(type,text) {
            sessionStorage.setItem("reloadMsg",JSON.stringify({type: type, text: text}));
        },
        setCookie: function(name,val,expires_in,path) {
            utils.setCookie(name,val,expires_in,path);
        },
        removeCookie: function(name, path) {
            utils.removeCookie(name, path);
        },
        blockResubmit: utils.blockResubmit
    };
}
http://requirejs.org/docs/errors.html#mismatch
    at makeError (require.js:166:17)
    at intakeDefines (require.js:1236:36)
    at require.js:1423:25Understand this error
/explore/manifest/js/utils.js:1 
        
        
       Failed to load resource: the server responded with a status of 404 ()Understand this error
manifest/:1 Refused to execute script from 'https://expr-dot-idc-dev.appspot.com/explore/manifest/js/utils.js' because its MIME type ('text/html') is not executable, and strict MIME type checking is enabled.Understand this error
require.js:141 Uncaught Error: Script error for: utils
http://requirejs.org/docs/errors.html#scripterror
    at makeError (require.js:166:17)
    at HTMLScriptElement.onScriptError (require.js:1696:36)Understand this error
/explore/manifest/js/libs/underscore-min.js:1 
        
        
       Failed to load resource: the server responded with a status of 404 ()Understand this error
manifest/:1 Refused to execute script from 'https://expr-dot-idc-dev.appspot.com/explore/manifest/js/libs/underscore-min.js' because its MIME type ('text/html') is not executable, and strict MIME type checking is enabled.Understand this error
require.js:141 Uncaught Error: Script error for: underscore
http://requirejs.org/docs/errors.html#scripterror
    at makeError (require.js:166:17)
    at HTMLScriptElement.onScriptError (require.js:1696:36)Understand this error
/explore/manifest/js/libs/jquery-3.7.1.min.js:1 
        
        
       Failed to load resource: the server responded with a status of 404 ()Understand this error
manifest/:1 Refused to execute script from 'https://expr-dot-idc-dev.appspot.com/explore/manifest/js/libs/jquery-3.7.1.min.js' because its MIME type ('text/html') is not executable, and strict MIME type checking is enabled.Understand this error
require.js:141 Uncaught Error: Script error for: jquery
http://requirejs.org/docs/errors.html#scripterror
    at makeError (require.js:166:17)
    at HTMLScriptElement.onScriptError (require.js:1696:36)Understand this error
require.js:141 Uncaught Error: Load timeout for modules: jqueryui,bootstrap,session_security,assetscore,assetsresponsive,tablesorter
http://requirejs.org/docs/errors.html#timeout
    at makeError (require.js:166:17)
    at checkLoaded (require.js:692:23)
    at require.js:713:25Understand this error
manifest/:1         
        
       Failed to load resource: the server responded with a status of 500 ()

• nitpick: why is the cart page theme in purple, while the explore page is in black (header, icons color). Is this intentional? I would expect the color schemes to be synchronized.

• after adding some number of collections to the cart, going to the cart to see the content gives me a lengthy spinner followed by the error.

[G-White-ISB]

1. We can rephrase the cart tooltip as you like. We could also move the cart summary out of a tooltip and onto the page. But I should mention we had previously considered putting the cohort summary in a tooltip. We can include file sizes.

2. Sorry I should have mentioned that cart manifest download was not working. It needs to be integrated with changes Suzanne is making to cohort manifest download

3. Please compare the cart page to the collections page and the cohorts page. They share the same style. explore page is different because it does not consist of a single table like these other pages

4. I am trying to track down that error you received fetching the cart data. If you can replicate please let me know.

[fedorov]

We can rephrase the cart tooltip as you like. We could also move the cart summary out of a tooltip and onto the page. But I should mention we had previously considered putting the cohort summary in a tooltip. We can include file sizes.

Ok, let's rephrase the tooltip as I suggested and include total size: ""X series selected from K collections/L cases/M studies. Total size N GB/TB."

Please compare the cart page to the collections page and the cohorts page. They share the same style. explore page is different because it does not consist of a single table like these other pages

Let me rephrase the question. Is there any logic in having table in pink in the cart page, and black in the explore page? I have nothing against pink. Just curious about the motivation for using different colors.

I am trying to track down that error you received fetching the cart data. If you can replicate please let me know.

I've just tried again, and was able to reproduce it on the first try by adding NLST collection to the cart.

[fedorov]

Few nitpicks:

1. replicate cart summary on the cart page
2. add button "Open cart" with the card icon in the upper right
3. fix tall line height in the cart page

[G-White-ISB]

@fedorov, all items in the comment directly above have been fixed. Also, the cart summary text has been changed to your recommendation, except size is not explicitly calculated. The size calculation is being considered.

[fedorov]

As communicated on slack, trying to add NLST to the cart results in a ~20 sec delay followed by empty cart.

Regarding the revised design, here's the initial mockup (colors/icons are placeholders, open for revision) (source https://drive.google.com/file/d/1UDZlab52Q1Ly3Q-j7kykmHgVqgs5oKsn/view?usp=sharing)

We could have filter cohort and cart cohort to be presented on separate, collapsible panels.

[G-White-ISB]

What is that icon just to the LHS of the Filter cohort definition? Is that clickable?

[fedorov]

No, it's not a button, just an icon. I just had this idea to connect the concept of "filter cohort" with an intuitive icon, to parallel that for the cart cohort. Just an idea.

[fedorov]

@G-White-ISB I tried again today. This time it took about 50 seconds! But this time the cart was not empty when spinner was done.

But why is the cart icon yellow, if I added the entire collection?

[fedorov]

Another observation/concern. If I try to remove an individual series from the cart after adding the entire NLST collection, after I click the remove from cart button at the series level, I do not get any spinner, but I also cannot interact with the page for about 25 seconds. At the very least, there should be a spinner, no?

[G-White-ISB]

When you add NLST to your cart in the test portal the back finds ONE LESS series to add than is expected, 587771 instead of 587772. That is why the cart is yellow. This problem does not occur in the expr portal where all 587772 series are added to the cart. I'm confused by this and may need Suzanne's help.

When you remove or add a case/study/series the cart counts on the tables, the cart colors, functions of the carts (add or remove) are all updated. I have a generic function to take care of this but I realize now its not efficient for a large cart. It wouldn't be hard to retool and would be worth doing. Also, agree a spinner would be useful. Its a front end calculation so there is no sever call to automatically start the spinner

[fedorov]

@G-White-ISB following up on the discussion this week, please return the capability that allows to expand a row in a table by clicking anywhere in the row (except the clipboard copy buttons etc) to make the new behavior consistent with the current behavior of the portal.

[fedorov]

@s-paquette bringing back the functionality to allow clicking on the row should not be hard. If it turns out to be difficult, we will revisit.

[fedorov]

Tooltips for cart should depend on the level. For collection level it is currently shows "add series".

[fedorov]

Spinner on operations is still missing (e.g., removing a single series from the cart).

Sometime spinner is present, then disappears, but some call is still not done.

[fedorov]

@s-paquette I realized that lagginess in updating the UI and lack of UI blocking until update is completed that we observe for the test tier as part of cart testing right now is the same issue as reported here: #1420.

[s-paquette]

@fedorov Yes these are largely all related at this point.

Builds are now up on dev and test which address the following:

• Larger chevron
• Chevron background is colored based on hover and open/closed status
• 'large' (i.e. via BQ) cart downloads available on cart page

Investigating: single-series addition seems to break under some conditions

Question: @fedorov In the case where eg. a study, case, or collection is added to the cart, but some of the series contained in those do not match the filter (eg. perhaps their modality is different) - does the filter take precedence over the cart addition?

[fedorov]

Good question! Let's add this to the agenda for the tomorrow meeting. I think it can be one way or another, and I am not sure which way is more intuitive.

[G-White-ISB]

With respect to this question: on June 20 in the SLACK releases-dev Channel:

George White:
"But also.. if you filter first, say on Modality = Radiotherapy Structure Set, and then click the cart in the 4D_Lung row you will only add the 1600 studies in that collection associated with that modality. That is how TCIA works. Alternatively one could decide that clicking the cart for 4D_Lung adds ALL 6690 in that collection REGARDLESS of the current filter definition. I'm not sure which behavior would be more expected"

David Gibbs: "I think we should stick with our current system, making selections available as defined by the current filter. 1. Set filter, 2. make selections under that filter."
Andrey Fedorov gives a thumbs up to David Gibbs comment.

[s-paquette]

@fedorov @pgundluru New build on dev and test which addresses some of the button moving/adding, etc.

Andrey, to make sure I have this right, you want the cart info and filter info int their own distinct panels, right? We could swap the filter panel to behave like PDC's: they don't have a title on it, instead while there's no filter chosen it just says 'select a filter at the left' and once you do, the action buttons and filter def appear. Does this sound preferable? It saves us some space and looks a bit cleaner overall.

Then, for the cart, similarly while it's empty it's just a bit of text saying 'cart empty. use the cart icons in the table below to add series to this cart' or similar. Then once something is added, the descriptive text and buttons appear.

[fedorov]

Andrey, to make sure I have this right, you want the cart info and filter info int their own distinct panels, right?

Yes, I would like to experiment with that, if this is not difficult. Can you do this, so I can tell if this looks good? Please ping me on slack if you can when it is ready.

We could swap the filter panel to behave like PDC's: they don't have a title on it, instead while there's no filter chosen it just says 'select a filter at the left' and once you do, the action buttons and filter def appear. Does this sound preferable? It saves us some space and looks a bit cleaner overall.
Then, for the cart, similarly while it's empty it's just a bit of text saying 'cart empty. use the cart icons in the table below to add series to this cart' or similar. Then once something is added, the descriptive text and buttons appear.

Yes, let's try this out!

Couple other cosmetic suggestions that I hope are quick to implement:

• Could you try adding a download button for both the cart and cohort filter along with the buttons/filter definition? Perhaps using the same icon we use down at the study level for downloading? Right now the download buttons are in the top right and separate from the cohort/cart definition, which is not intuitive. We can keep or remove the current cohort/cart download buttons.
• I suggest we replace the text "Filter definition" with "Cohort filter definition". Otherwise, "Filter Definition" text is disconnected with "Cohort Download" button.
• As discussed earlier, can you please add clear cart button to the left of the view cart? It can be the same icon as reset of the cohort filter.
• Can you please either hide the "info" button in the right-most part of the collapsed charts area when that area is collapsed, or if it is not possible to hide it, just remove it? It is confusing the way it is right now.

Also, I noticed expansion of the hierarchy down from the collection level is super slow, and the issue with UI update lag is very prominent. Please see this video: https://app.screencast.com/XO3w2UtNW62Zj. This is extremely confusing and not acceptable to pass testing! The delay in UI update is also not acceptable. Is it really our only option to test if we have the same issue in production tier by moving it to prod?

[s-paquette]

@fedorov A note, the screencast has no audio for me; I don't know if it was supposed to, but I don't hear anything. It looks like in the video that you started with a cart already selected--can you let me know what series was in there before you started this? (I can't always reliably reproduce the behaviors without following the exact route taken, so it's best if I can start from the cart you have here in the video.)

The rest of this is all very doable. One point, I'd mentioned removing 'Filter Definition' entirely, since we're splitting cart contents and filter definition from one another there's maybe less reason to title anything. Did you want to keep those titles per your second bullet item?

[fedorov]

@fedorov A note, the screencast has no audio for me; I don't know if it was supposed to, but I don't hear anything. It looks like in the video that you started with a cart already selected--can you let me know what series was in there before you started this? (I can't always reliably reproduce the behaviors without following the exact route taken, so it's best if I can start from the cart you have here in the video.)

There was no audio. Here's a new video that starts from empty: https://app.screencast.com/DgSCfrnFkCESH. In summary: 1) add single case from NLST to the cart; 2) observe enormous (~20 sec!) delays in updating the content of the collections list after resetting/changing the filter (and that is ~20 sec after the spinner disappears).

I'd mentioned removing 'Filter Definition' entirely, since we're splitting cart contents and filter definition from one another there's maybe less reason to title anything. Did you want to keep those titles per your second bullet item?

If we move the download button into the same tile as the cohort filter definition, then we can remove the title. Otherwise I think it is too difficult to connect the unnamed filter definition with the "Download cohort" button.

[s-paquette]

@fedorov Got it, and thanks for the new video. Here's what I have at the moment:

One catch. View cart, unlike ALL the other buttons, redirects to a new page. Making it open a dialog would only slow the page down more, and making it open in a new tab (as opposed to the current one) would take some time to rewrite. Are we okay with this?

An upshot/stopgap is you can hit the back button from the cart and you will be back on the explorer page with no changes or reloads.

Note, this image above is currently building on dev and test, will be available in ~20m

[fedorov]

Here's what I have at the moment:

I feel like something is missing. Since we have 2 tiles, I feel like they need some titles. But I don't want to delay. We can refine later.

BUT few small things would be nice to fix:

• color of text with the content of cohort/cart should be consistent (black)
• cart empty button does not work
• can we change the text to "Filter cohort content: [blah]" and "Cart content: [blah]", and also make sure that "[blah]" is formatted consistently between cohort and cart (with the exception that as discussed, we cannot report size of the cart in TB)? I suggest this format: "X series from Y collections / Z cases / N studies". Right now the order, capitalization and separators are different between the two.
• charts info button is still there in the collapsed charts panel

View cart, unlike ALL the other buttons, redirects to a new page. [...] Are we okay with this?

Yes, in spirit of being agile, I am ok with this!

[s-paquette]

@fedorov Note, we don't currently tabulate the collections which are in a cohort. Although we can this requires some changes to how counts are done on the backend, so I'd like to recommend we break that specific item out into a separate ticket ('include number of collections in cohort stats' or similar) and otherwise I can make them all look the same.

[fedorov]

Ok, let's then align formatting while skipping collection count and total size for the cart content.

[s-paquette]

@fedorov Most recent build has addressed: info tooltip icon being hidden when the charts are collapsed, formatting and colors on the cart and cohort information text, button activities.

Remaining: removing/adding a specific series is causing an error when you attempt to view the cart, and the slowdowns with large carts.

[s-paquette]

@fedorov @pgundluru Dev tier now has the following:

• When the page first loads, the shift from Alphabetical sorting to Numeric will engage the spinner to prevent confusing clicks. Note that on a sufficiently fast browser or machine this can result in a little bit of a flicker.
• Expanding table contents will now engage the spinner
• Clicking on the cart icon to add or remove collections, cases, studies, and/or series should engage the spinner. I tested this using NLST, which was the slowest performing when adding/removing from the cart. Note the slowness is still present, and I have ideas on how to improve that. Let me know if you don't see a spinner when adding to/removing from the cart, as it wasn't consistent in my initial tests but seems to be now.
• Cart manifest downloads should be working, both as immediate downloads and delayed downloads for larger manifests.

[s-paquette]

Planned for when I get back: basic visual adjustments so it's not just the monolithic spinner no matter what anyone does, i.e., 'Updating cart...' for cart operations, 'Updating tables...' for table operations, this kind of thing.

[fedorov]

@s-paquette sounds great - let's advance to test!

[fedorov]

@s-paquette it appears that there is a long delay after resetting cohort filter and updating the content of the tables. There is no spinner while that is underway.

To reproduce this, select "ESophagus" under Derived > Segmentations > Segmentation type, add to cart some patients/studies from NLST (but not necessarily the entire collection), then reset cohort filter - list of collections update is lagging, while user can still interact with the items in the collections list.