Results for MDL-45041

• Remote repository: git://github.com/jleyva/moodle.git

• Remote branch MDL-45041 to be integrated into upstream master
  • Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/3089
  • Error: The MDL-45041 branch at git://github.com/jleyva/moodle.git does not apply clean to master

Results for MDL-45041

• Remote repository: git://github.com/jleyva/moodle.git

• Remote branch MDL-45041 to be integrated into upstream master
  • Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/3091
  • Details: http://integration.moodle.org/job/Precheck%20remote%20branch/3091/artifact/work/smurf.html

Hi Juan,
The patch looks good.
You have also removed the only usage of query that was filtering on the indexed columns, so should be safe to delete the index.
Does it need backporting? Also can you please mention in the testing instructions to verify the index was deleted in database.

On an un-related note, we should be using $DB->record_exists instead of get_record for those checks.

Cheers.

Submitting for integration, I've rebased.

This bug doesn't need backporting

Testing instructions updated

Thanks Juan, I added a label integration_held and it will be integrated after the end of on-sync period (in two weeks). You will need to rebase again and change version number then. Also think if you want to retain this index without unique-constraint to use in select queries by those two fields.

The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

TIA and ciao

Moving this issue to current integration cycle, will be reviewed soon. Thanks for the hard work!

Hi Juan,

1. I was talking with Eloy about this and he recommended the use of drop_key() rather than drop_index() - which will ensure in future if we enforce the referential integrity that things get sorted out properly
2. What is not entirely clear from this issue is what the use case is for two users with the same push key. Could explain this more? Thinking about it from the iOS point of view, it might be quite confusing if multiple users are logged into the same device - especially shared devices...

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

Hi Dan,

1. Thanks, I'll change it

2. Currently the app supports multiple sites for multiple users.
So imagine that you have two user accounts in the same Moodle installation, you are going to have always the same Push Token (pushid) since is unique per device/app.
Once the app receives a notification it can detect the site/user where it came from but if the app is in background the notification "short text" is going to be displayed in the notifications tray.

We are aware of the problem of shared devices, it's possible to receive notifications from different sites in a shared device so a user is going to receive notifications from everywhere, in that case, the user should configure his message settings in order to avoid receiving Push notifications (or just to avoid receive private messages etc... )

I think this is something that should be explained in the app docs

+1 code verified against automated checks.

Checked MDL-45041 using repository: git://github.com/jleyva/moodle.git

• MOODLE_26_STABLE (branch: MDL-45041_26_STABLE | CI Job)
• MOODLE_27_STABLE (branch: MDL-45041_27_STABLE | CI Job)
• master (branch: MDL-45041 | CI Job)

_{More information about this report}

Moving this issue to current integration cycle, will be reviewed soon. Thanks for the hard work!

Thanks Juan this has been integrated now.

Thanks for working on this, Juan! Tests passed on 2.6, 2.7 and master.

Thanks for your contribution - this change is now part of Moodle!

True knowledge exists in knowing that you know nothing.
– Socrates