Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Activity
    Dan Gray
    @dancgray
    That sounds less than ideal. I though that was only for master tenants?
    Louise McMahon
    @canstudios-louisem
    ill raise it on the pr
    Louise McMahon
    @canstudios-louisem
    Ive raised the tenant admin plugin install issue on the pr https://github.com/adaptlearning/adapt_authoring/pull/1637#issuecomment-304897489
    Also whats the best way to discuss the db issue, we said it needed further discussion?
    Tom Taylor
    @taylortom
    Excellent, thanks @canstudios-louisem
    It may be best to arrange a call, my main reason for halting the discussion today was that @aniketdharia and @ambadasavhad were missing
    Failing that, we could always do it in a blog post on the tech forum
    Aniket Dharia (aptlogica)
    @aniketdharia
    Hi @taylortom amabadas is struggling with access issue to commit the latest code. Can you resolve it?
    Tom Taylor
    @taylortom
    Hi Aniket, yeah sure, what’s the problem?
    Ambadas Avhad
    @ambadasavhad
    @aniketdharia problem is resolved , @taylortom can you please review the changes
    Aniket Dharia (aptlogica)
    @aniketdharia
    oops! Just came in office .. :P tght it is still open...
    regarding new requirements.. can we have a short hangouts call @taylortom and @ambadasavhad ?
    Tom Taylor
    @taylortom
    Yeah sure, we suggested as much yesterday
    Louise McMahon
    @canstudios-louisem
    just had a quick look at the latest commit looks good @ambadasavhad are you planing to add some server side checks for if the user has permission to create a user in that tenant as not everything supports disabled and you could bypass that easily anyway?
    Ambadas Avhad
    @ambadasavhad
    @canstudios-louisem sure
    Tom Taylor
    @taylortom
    @canstudios-louisem there should be server-side validation for those routes, returning a 500 or similar (haven’t tested this recently though...)
    Louise McMahon
    @canstudios-louisem
    @taylortom Currently a tenant admin can put someone into a tenant that is not theirs which they shouldnt be able to do thats what I was getting at
    Tom Taylor
    @taylortom
    Ah ok, well that needs looking at then
    Louise McMahon
    @canstudios-louisem
    :)
    same should be done for the super admin role aswell
    Tom Taylor
    @taylortom
    @aniketdharia shall we pencil in a catch-up meeting some time this week?
    Louise McMahon
    @canstudios-louisem
    Im starting to struggle to track the issues we have raised for tenant management can i suggest instead of putting them on the pr we raise them as issues and add them to the tenant management milestone?
    Dan Gray
    @dancgray
    :+1: probably would be easier
    I think your point about XSS and handlebars should be raised as a seperate issue. It’s something we should address and apply consistently.
    Louise McMahon
    @canstudios-louisem
    I think there should be one issue for the tenant admin one and another for the other areas ill raise them though, it looks like QA has found a few in other areas aswell so it does need looking at.
    Dan Gray
    @dancgray
    :+1:
    canstudios-nicolaw
    @canstudios-nicolaw

    Afternoon all,

    We've been working on the tenant management branch here and have a few fixes ready for the todo items on the tenant management PR (adaptlearning/adapt_authoring#1637):

    • Tenant admin: Able to change the owner of any course in the tenancy
    • Tenant admin: Should not be able to upload plugins

    As part of the work to allow a tenant admin to change the owner of any course in their tenancy the tenant admin can see all courses in their tenant. This means that they can also delete courses in their tenant. However it seems to be the case that shared courses can only be deleted by the project owner - so a small change would be needed here for the following todo item to be checked off "Tenant admin: Able to remove any course in the tenancy".

    We have also been working on the naming of a tenant database. Currently the "Tenant name" field is used as the database name. There is a scenario which can cause a database clash:

    • Create a tenant with name "TenantA", this tenant will have a database of the same name
    • Change the tenant name, the name of it's database will not change
    • Create a second tenant with name "TenantA". This tenant will be created because the tenant name is unique, however it will share a database with the existing tenant.

    We have made a change so that the tenant id is used as the database name. This way it will always be unique and we don't have to worry about sanitising user input before it is used as the db name or scenarios like the above.

    We would like to give these back to the tenant management branch but aren't sure on the best way to do it. Should we create a PR to the branch or just add the changes to it? They are just being tested and should be ready sometime tomorrow.

    Thanks

    canstudios-nicolaw
    @canstudios-nicolaw
    ^ We have finished testing and are ready to give these back to the tenant management branch as soon as we know the best way to do it.
    Dan Gray
    @dancgray
    @canstudios-nicolaw if it was me I’d make a PR to tenant-management. But I have not been involved in the tenant management work much.
    Hope that helps :worried:
    BTW shoule creating a non master tenant create a new DB?
    canstudios-nicolaw
    @canstudios-nicolaw

    @dancgray I've noticed that when you first make a tenant its db doesn't appear in the db list. You have to create a user in that tenant and then that user needs to create something which would be stored in the tenant's db (eg. a course).

    All users are stored in the master db, so just adding a user to a tenant won't be enough to 'create' the tenant db.

    Hope that helps

    Aniket Dharia (aptlogica)
    @aniketdharia
    @/all Apologies for missing the call. @ambadasavhad will send the updated PR by this weekend.
    Dan Gray
    @dancgray
    Hi, is anyone else able to run install on the tenant-management branch. I'm getting ENOENT errors on the temp folder.
    Ambadas Avhad
    @ambadasavhad
    Hi @/all For update tenant list dropdown on adding new tenant in add user form , I think we have to make user-management dependable on tenant-management.
    any ideas?
    Tom Taylor
    @taylortom
    @ambadasavhad what do you need from tenant management?
    Ambadas Avhad
    @ambadasavhad
    @/all should users from non-master tenant able to upload the plugins?
    Tom Taylor
    @taylortom
    To me, it makes sense to restrict this to super admins only
    lc-thomasberger
    @lc-thomasberger
    I think we should support tenant specific plugins, as it is very likely that each tenant would have different themes.
    Louise McMahon
    @canstudios-louisem
    That was discussed origionally @lc-thomasberger but it was decided it was not MVP we should stick to what we decided earlier
    lc-thomasberger
    @lc-thomasberger
    Yes, I know that it is not part of MVP and that's fine for me. I just think that it is important for tenants to support different plugins. So in the long run we should support it, but not for MVP.
    Louise McMahon
    @canstudios-louisem
    Oh yes i agree its someting Can would need before we could really use multi tennant
    lc-thomasberger
    @lc-thomasberger
    :+1:
    Ambadas Avhad
    @ambadasavhad
    So for this MVP we need to hide either 'plugin management' from global menu or 'upload plugin' from plugin management sidebar for users other than 'super admin'. right?
    Louise McMahon
    @canstudios-louisem
    I think just hide it from the bar if your not a super admin in the master tennant.
    What do we want to do with import? curently thats limited to super admins as you can upload plugins via that.
    We talked about allowing none super admins to import but only if the course did not need to add a plugin
    Tom Taylor
    @taylortom
    Yeah, did we outline any requirements for import? Can’t remember now
    Louise McMahon
    @canstudios-louisem
    I dont think so as i dont think we expected it to be out first