Moodle
  1. Moodle
  2. MDL-34275

Use default copyright license when restoring files

    Details

    • Testing Instructions:
      Hide
      1. Create a course backup with files without license metadata (for example a Moodle 1.9 backup
      2. Restore that course backup into Moodle 2.3+ with the patch installed
        1. Verify that the restored files are using the site configured default license
      Show
      Create a course backup with files without license metadata (for example a Moodle 1.9 backup Restore that course backup into Moodle 2.3+ with the patch installed Verify that the restored files are using the site configured default license
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull Master Branch:
      MDL-34275-master

      Description

      Currently when you restore an M19 backup or an M2 backup that, for some reason, has license field empty the default copyright license isn't being used.

        Gliffy Diagrams

          Activity

          Hide
          Rex Lorenzo added a comment -

          For our default license we have "tbd".

          Show
          Rex Lorenzo added a comment - For our default license we have "tbd".
          Hide
          Rex Lorenzo added a comment -

          Rebased with the latest master.

          Show
          Rex Lorenzo added a comment - Rebased with the latest master.
          Hide
          Dan Poltawski added a comment -

          Hi Rex,

          Looks good to me. The only thing is that the null value should be lowercase according to our coding style:
          http://docs.moodle.org/dev/Coding_style#Booleans_and_the_null_value

          If you could create testing instructions and branches for stables I think we could get this submitted for integration.

          [N] Syntax
          [-] Output
          [Y] Whitespace
          [-] Language
          [-] Databases
          [N] Testing
          [-] Security
          [-] Documentation
          [Y] Git
          [Y] Sanity check

          Show
          Dan Poltawski added a comment - Hi Rex, Looks good to me. The only thing is that the null value should be lowercase according to our coding style: http://docs.moodle.org/dev/Coding_style#Booleans_and_the_null_value If you could create testing instructions and branches for stables I think we could get this submitted for integration. [N] Syntax [-] Output [Y] Whitespace [-] Language [-] Databases [N] Testing [-] Security [-] Documentation [Y] Git [Y] Sanity check
          Hide
          Rex Lorenzo added a comment - - edited

          Fixed NULL to be null. Added patches for 2.3/2.4 stable branches. The cherry-pick for 2.2 was not successful and the code changed a bit since that version. Is it okay to skip applying a patch for that version?

          Show
          Rex Lorenzo added a comment - - edited Fixed NULL to be null. Added patches for 2.3/2.4 stable branches. The cherry-pick for 2.2 was not successful and the code changed a bit since that version. Is it okay to skip applying a patch for that version?
          Hide
          Dan Poltawski added a comment -

          Yep, in fact its encouraged since we are only supporting 2.2 for security fixes.

          Show
          Dan Poltawski added a comment - Yep, in fact its encouraged since we are only supporting 2.2 for security fixes.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Sorry, I'm reopening this, incorrect whitespace present.

          Show
          Eloy Lafuente (stronk7) added a comment - Sorry, I'm reopening this, incorrect whitespace present.
          Hide
          Dan Poltawski added a comment -

          I should've caught that, musn't have pulled it in

          Show
          Dan Poltawski added a comment - I should've caught that, musn't have pulled it in
          Hide
          CiBoT added a comment -

          Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

          Show
          CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
          Hide
          Rex Lorenzo added a comment -

          Fixed whitespace issue. Submitting for peer review/integration.

          Show
          Rex Lorenzo added a comment - Fixed whitespace issue. Submitting for peer review/integration.
          Hide
          Dan Poltawski added a comment -

          Hi Rex,

          This still seems to have the whitespace issue.

          Show
          Dan Poltawski added a comment - Hi Rex, This still seems to have the whitespace issue.
          Hide
          Dan Poltawski added a comment -

          I've fixed up the patch for you and sending it to integration

          Show
          Dan Poltawski added a comment - I've fixed up the patch for you and sending it to integration
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated (23, 24 & master), thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated (23, 24 & master), thanks!
          Hide
          Rossiani Wijaya added a comment -

          This is working as expected.

          Tested for 2.3, 2.4 and master

          Test passed.

          Show
          Rossiani Wijaya added a comment - This is working as expected. Tested for 2.3, 2.4 and master Test passed.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Did you think this day was not going to arrive ever?

          Your patience has been rewarded, yay, sent upstream, thanks!

          Closing...ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Did you think this day was not going to arrive ever? Your patience has been rewarded, yay, sent upstream, thanks! Closing...ciao
          Hide
          Andrew Nicols added a comment -

          I'm just working in this neck of the woods, and notice that the default license isn't applied to file aliases - is this deliberate?

          Show
          Andrew Nicols added a comment - I'm just working in this neck of the woods, and notice that the default license isn't applied to file aliases - is this deliberate?
          Hide
          Rex Lorenzo added a comment -

          The fix is in place mainly for when you restore a M1.9 backup. Before the fix, those files get no copyright license at all. Now, they will import with the default license.

          Show
          Rex Lorenzo added a comment - The fix is in place mainly for when you restore a M1.9 backup. Before the fix, those files get no copyright license at all. Now, they will import with the default license.
          Hide
          Andrew Nicols added a comment -

          Ah - I see, so it only affects conversions from 1.9, and since aliases are a 2.X feature only, there's no need for a default license there.

          Cheers.

          Show
          Andrew Nicols added a comment - Ah - I see, so it only affects conversions from 1.9, and since aliases are a 2.X feature only, there's no need for a default license there. Cheers.
          Hide
          Mary Cooch added a comment -

          Removing docs_required as I think this expected behaviour doesn't need documenting but please say if you disagree.

          Show
          Mary Cooch added a comment - Removing docs_required as I think this expected behaviour doesn't need documenting but please say if you disagree.

            People

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

              Dates

              • Created:
                Updated:
                Resolved: