Moodle
  1. Moodle
  2. MDL-33777

Equella repository needs to set field-source field correctly and use it to indicate source file too

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: 2.3
    • Component/s: Repositories
    • Labels:
    • Rank:
      41823

      Description

      Following on from MDL-33513

      repository/equella/lib.php needs two new functions:

      function get_file_source_info($filepath)
      .... returns 'Equella: originalfilename.ext' which came from filepicker which may not be the current name in Moodle.

      function get_reference_details($reference, $filestatus = 0)
      .... returns the data from the associated file->source field

      (I just tried to do this quickly myself but I'm getting a bit lost in the repository woods!)

        Issue Links

          Activity

          Hide
          Martin Dougiamas added a comment -

          Dongsheng can you please look at this too? It was missing from the other patch MDL-33513

          Show
          Martin Dougiamas added a comment - Dongsheng can you please look at this too? It was missing from the other patch MDL-33513
          Hide
          Dongsheng Cai added a comment -

          Sure

          Show
          Dongsheng Cai added a comment - Sure
          Hide
          Martin Dougiamas added a comment -

          Thanks DS. Nick, does this look OK to you?

          Show
          Martin Dougiamas added a comment - Thanks DS. Nick, does this look OK to you?
          Hide
          Nick Read added a comment -

          It does for now given the limited time. After 2.3 there's a couple of areas like this that should change away from doing URL grabs like this (although this one is only a HEAD which is great) and instead using the information returned from EQUELLA during the original selection. That's a bigger fix that is capable for this release though.

          Show
          Nick Read added a comment - It does for now given the limited time. After 2.3 there's a couple of areas like this that should change away from doing URL grabs like this (although this one is only a HEAD which is great) and instead using the information returned from EQUELLA during the original selection. That's a bigger fix that is capable for this release though.
          Hide
          Dongsheng Cai added a comment -

          Wait a second, I didn't look the ticket carefully yesterday, I didn't add "get_reference_details" method, I'm gonna do this now.

          Show
          Dongsheng Cai added a comment - Wait a second, I didn't look the ticket carefully yesterday, I didn't add "get_reference_details" method, I'm gonna do this now.
          Hide
          Dongsheng Cai added a comment -

          repository_equella::get_reference_details() method added.

          Show
          Dongsheng Cai added a comment - repository_equella::get_reference_details() method added.
          Hide
          Martin Dougiamas added a comment - - edited

          Dongsheng, for future reference ... why are you grabbing the data like this in get_file_source_info()?

          Is it not possible to save the original name passed back to the filepicker? That's where I got lost, trying to find that point.

          Show
          Martin Dougiamas added a comment - - edited Dongsheng, for future reference ... why are you grabbing the data like this in get_file_source_info()? Is it not possible to save the original name passed back to the filepicker? That's where I got lost, trying to find that point.
          Hide
          Martin Dougiamas added a comment -

          Also, now I'm looking at the new patch ... having to call CURL every time we call either of these statements is really scary. That was the whole point of storing it in files->source field ... so we could refer to it in future (it doesn't change).

          Show
          Martin Dougiamas added a comment - Also, now I'm looking at the new patch ... having to call CURL every time we call either of these statements is really scary. That was the whole point of storing it in files->source field ... so we could refer to it in future (it doesn't change).
          Hide
          Dongsheng Cai added a comment -

          Martin

          It's not possible at the moment, the filename isn't sent back to repository_ajax.php, in the future, we could send original file name to repository_ajax.php when click the select button if it exists.

          Show
          Dongsheng Cai added a comment - Martin It's not possible at the moment, the filename isn't sent back to repository_ajax.php, in the future, we could send original file name to repository_ajax.php when click the select button if it exists.
          Hide
          Dongsheng Cai added a comment -


          Also, now I'm looking at the new patch ... having to call CURL every time we call either of these statements is really scary. That was the whole point of storing it in files->source field ... so we could refer to it in future (it doesn't change).

          Martin, that's a HEAD request, no http body will be returned. We are not able to access source filed in get_reference_details method because we don't have stored_file instance here.

          Show
          Dongsheng Cai added a comment - Also, now I'm looking at the new patch ... having to call CURL every time we call either of these statements is really scary. That was the whole point of storing it in files->source field ... so we could refer to it in future (it doesn't change). Martin, that's a HEAD request, no http body will be returned. We are not able to access source filed in get_reference_details method because we don't have stored_file instance here.
          Hide
          Marina Glancy added a comment -

          do we really need to send http request to get the name of the source file?
          can't we store this information?

          for exmple, just retrieve this once and store in files.source
          and every time we need get_reference_details parse it from there

          Show
          Marina Glancy added a comment - do we really need to send http request to get the name of the source file? can't we store this information? for exmple, just retrieve this once and store in files.source and every time we need get_reference_details parse it from there
          Hide
          Dan Poltawski added a comment -
          1. Whitespace:
            https://github.com/dongsheng/moodle/compare/integration...dev_MDL-33777_equella_sourcefield#L0R367
          2. We already discovered that using $USER unchecked causes problems, please avoid that.
          3. This 'grab_filename_from_url' looks really ugly. This should be coming from equella as metadata not hacking around with the http request.

          I don't like this at all.

          Show
          Dan Poltawski added a comment - Whitespace: https://github.com/dongsheng/moodle/compare/integration...dev_MDL-33777_equella_sourcefield#L0R367 We already discovered that using $USER unchecked causes problems, please avoid that. This 'grab_filename_from_url' looks really ugly. This should be coming from equella as metadata not hacking around with the http request. I don't like this at all.
          Hide
          Martin Dougiamas added a comment -

          Marina/Dan - that's what I said too, but DS says not possible.

          Show
          Martin Dougiamas added a comment - Marina/Dan - that's what I said too, but DS says not possible.
          Hide
          Dan Poltawski added a comment -

          Integrated and testing.

          Show
          Dan Poltawski added a comment - Integrated and testing.
          Hide
          Dan Poltawski added a comment -

          Tested during integration.

          Show
          Dan Poltawski added a comment - Tested during integration.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Yay. just in time for Moodle 2.3 release! Many thanks!

          Closing as fixed, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Yay. just in time for Moodle 2.3 release! Many thanks! Closing as fixed, ciao

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: