Ticket #8264 (closed: fixed)

Opened 7 years ago

Last modified 5 years ago

Disable icat download button if user has access to the archives

Reported by: Jay Rainey Owned by: Jay Rainey
Priority: major Milestone: Release 3.2
Component: GUI Keywords: ICAT,ICAT4
Cc: Blocked By:
Blocking: Tester: Samuel Jackson

Description (last modified by Jay Rainey) (diff)

The functionality of the download button is to download the file from the archives IF the user does not have access. Therefore, if the user has access to the archives then the button should be disabled and a message informing them why when the datafile table is populated.

Change History

comment:1 Changed 7 years ago by Jay Rainey

  • Description modified (diff)

comment:2 Changed 7 years ago by Jay Rainey

  • Milestone changed from Release 3.1 to Backlog

comment:3 Changed 7 years ago by Jay Rainey

  • Summary changed from [ICAT] Disable download button to Disable icat download button

comment:4 Changed 7 years ago by Jay Rainey

  • Milestone changed from Backlog to Release 3.2

comment:5 Changed 7 years ago by Nick Draper

  • Status changed from new to assigned

Bulk move of tickets out of triage (new) to assigned at the introduction of the triage state

comment:6 Changed 7 years ago by Jay Rainey

  • Status changed from assigned to inprogress

Disable download button if user has archive access. Refs #8264.

Changeset: d5a6ec21fcb91a217fd8ff5baa590cd6a012fd9c

comment:7 Changed 7 years ago by Jay Rainey

Use c string instead. Refs #8264.

Changeset: cabb1250b955b3b04d5e9c8c2215629f63e7cdf3

comment:8 Changed 7 years ago by Jay Rainey

  • Status changed from inprogress to verify
  • Resolution set to fixed

To test this ticket, you must have Ubuntu and have access to the archives.

Overview

Previously, if the user had access to the archives, they could click Download on a datafile in the Datafiles information table, and nothing would be downloaded. This ticket addresses this issue by disabling the download button if the user has access to the archives. This will prevent any confusing.

To test

Assuming you have access to the archives:

  1. Log into the catalog.
  2. Perform a search and select an investigation from the results and the Datafiles information: table will be populated.
  3. Select a datafile. The Download to... button will now be disabled.
  4. Verify this is true for the select all check-box.

Disable archive access

To disable the access to the archives on Ubuntu:

  sudo vim /etc/auto.master
  comment out line 24 (/archive /etc/auto.archive)
  sudo service autofs restart

Repeat the steps above to verify the Download to... button is now enabled when you select a datafile to download.

comment:9 Changed 7 years ago by Jay Rainey

  • Summary changed from Disable icat download button to Disable icat download button if user has access to the archives

comment:10 Changed 7 years ago by Keith Brown

  • Status changed from verify to verifying
  • Tester set to Keith Brown

comment:11 follow-up: ↓ 12 Changed 7 years ago by Keith Brown

  • Status changed from verifying to reopened
  • Resolution fixed deleted

Having seen the code it needs to check if the file exists first before trying to open it as a ifstream as this crashes on windows if it doesn't exist.

As although it said to test on ubuntu, it seems i was right to test it on windows (physically disconnecting myself and tethering to my phone in order to disconnect me from the networked archives) as when i click on an investigation's file and it checks for an archive connection it crashes all of mantid, and only responds again when an archive connection is restored.

comment:12 in reply to: ↑ 11 Changed 7 years ago by Jay Rainey

  • Status changed from reopened to verify
  • Resolution set to fixed
  • Tester Keith Brown deleted

Replying to Keith Brown:

Having seen the code it needs to check if the file exists first before trying to open it as a ifstream as this crashes on windows if it doesn't exist.

A check is already performed in each algorithm (and now the interface) to check if a file can be opened (if not, then it's invalid or does not exist).

As although it said to test on ubuntu, it seems i was right to test it on windows (physically disconnecting myself and tethering to my phone in order to disconnect me from the networked archives) as when i click on an investigation's file and it checks for an archive connection it crashes all of mantid, and only responds again when an archive connection is restored.

I was unable to reproduce the error described by Keith above. I attempted to reproduce this in two ways:

  1. Using W7 in a VM that does not have access to the archives.
  2. I manually set fileLocation to an invalid path in the interface class and the download algorithm on a machine that has access to the archives, which in turn disabled access.

The Download to... button was disabled when I had access to the archives, and enabled when I did not. To test this ticket see comment:8

Last edited 7 years ago by Jay Rainey (previous) (diff)

comment:13 Changed 7 years ago by Keith Brown

I've deemed the fault i experienced was due to extremely slow internet connection and the method of connection as i was tethering on my phone, so that could have caused the problem i experienced, as the fault occurred anywhere on my windows PC when i tried to navigate to a (non-existent) networked PC. However i could only test versus what i experienced.

When i said about a check I was meaning using something like Poco::File.exists() as I don't believe that accesses it, as although an ifstream does return false if it fails it does try to access the file. So i think exists() might be cheaper especially when the file isn't going to be read, like in both examples.

Last edited 7 years ago by Keith Brown (previous) (diff)

comment:14 Changed 7 years ago by Samuel Jackson

  • Status changed from verify to verifying
  • Tester set to Samuel Jackson

comment:15 Changed 7 years ago by Samuel Jackson

  • Status changed from verifying to closed

Merge remote-tracking branch 'origin/feature/8264_disable_icat_download'

Full changeset: 359b2362d41bb3032a2ba69ff59770c46cf7d870

comment:16 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 9109

Note: See TracTickets for help on using tickets.