Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Limit the licenses that can be linked to an article, by journal #713

Open
ajrbyers opened this issue Dec 17, 2018 · 9 comments
Open

Limit the licenses that can be linked to an article, by journal #713

ajrbyers opened this issue Dec 17, 2018 · 9 comments
Labels
dev-ready This issue has been refined and is ready for development. enhancement Add or improve something on an existing feature size M Medium

Comments

@ajrbyers
Copy link
Member

If an object has a journal we should limit the license queryset to licenses of that journal

@ajrbyers ajrbyers added the enhancement Add or improve something on an existing feature label Dec 17, 2018
@ajrbyers ajrbyers added this to the 1.4 (paris) milestone Dec 17, 2018
@ajrbyers
Copy link
Member Author

Adding this to Trello, please ignore.

@ajrbyers ajrbyers removed this from the 1.4 (paris) milestone May 28, 2020
@ajrbyers
Copy link
Member Author

@joemull can you fold this into your admin work?

@joemull joemull self-assigned this Nov 10, 2022
@ajrbyers ajrbyers moved this to Todo in v1.5 Torres Nov 21, 2022
@joemull joemull moved this from Todo to In Progress in v1.5 Torres Nov 21, 2022
@joemull joemull added the jy label Feb 7, 2023
@ajrbyers ajrbyers added size M Medium dev-ready This issue has been refined and is ready for development. and removed jy labels Feb 7, 2023
@joemull
Copy link
Member

joemull commented Feb 9, 2023

I'm finally getting around to this now. Can I clarify, @ajrbyers, which admin view does it relate to?

It may be we've achieved this by now in #3256. Here's what the Licence list view on that branch looks like, including the ability to filter by journal:
Screenshot from 2023-02-09 17-29-26

The foreign key field on Article and Preprint is also tagged as a raw_id_field, so it will bring up the same filterable list when you go to change the licence of an article or preprint:
Screenshot from 2023-02-09 17-31-25

@ajrbyers
Copy link
Member Author

ajrbyers commented Feb 9, 2023

Hey @joemull I was thinking if you're in admin, on a journal site and looking at, say, an article that the license dropdown should filter to the licenses that relate to that journal!

@joemull
Copy link
Member

joemull commented Feb 10, 2023

OK, got it. With this idea, I like that it would be easy to choose the relevant license, as long as you knew that was happening.

However, the user would not know a filter had been applied, and we don't apply a similar filter elsewhere now, AFAIK, so they wouldn't necessarily expect it. And a dropdown menu would give the string representation of the license, which does not include the journal name. I wish there were a way to give them this kind of info, with this idea.

For context, what I've done consistently in #3256 is make sure that if a foreign key field is journal-specific, it is marked as a raw_id_field so that the user is taken to the list view, where they have all the contextual info and filtering capability they need to choose the right license, provided they know the data model.

The other thing on my mind is consistency. If we did this for one foreign-key field on a journal-specific object, I think we'd want to do it for all of them, but that's not easy because not all journal-specific objects have a simple relation to a journal. We could try, by applying a custom form field to a base class that is applied for objects directly related to journal as well as ones tenuously related through another object.

In terms of user experience, if this were an end-user interface where the action would be routine and repetitive, I'd be much more willing to filter the license silently for the user, because it avoids a click and helps them choose the appropriate object for a data model they don't know the details of.

However, I think what we want in admin is comprehensive information and maximum control, since it is staff-facing, or ideally only administrator-facing. So, here I'd prioritise consistency, visibility of status, and control over click minimisation and automatic filtering.

What do you think?

@ajrbyers
Copy link
Member Author

ajrbyers commented Feb 10, 2023

This request was added before the field was a raw_id_field where it was literally impossible to know which licenses related to which journals. Having the ability to see the journal data is a definite step in the right direction in terms of usability.

However, I think what we want in admin is comprehensive information and maximum control, since it is staff-facing, or ideally >only administrator-facing. So, here I'd prioritise consistency, visibility of status, and control over click minimisation and >automatic filtering.

This still leaves the possibility that you will select license that isn't available on the journal your article belongs to, meaning that in the Editor interface it will show no license because that field is filtered.

An alternaitve is that we should validate the license on the article model to stop this from occuring but personally I think both filtering the list and validating the license would be the right move. And yes we probably should make this consistent across all of the FK fields in admin.

I think in this case data integrity trumps maximum control.

@mauromsl
Copy link
Member

OK, got it. With this idea, I like that it would be easy to choose the relevant license, as long as you knew that was happening.

However, the user would not know a filter had been applied, and we don't apply a similar filter elsewhere now, AFAIK, so they wouldn't necessarily expect it. And a dropdown menu would give the string representation of the license, which does not include the journal name. I wish there were a way to give them this kind of info, with this idea.

For context, what I've done consistently in #3256 is make sure that if a foreign key field is journal-specific, it is marked as a raw_id_field so that the user is taken to the list view, where they have all the contextual info and filtering capability they need to choose the right license, provided they know the data model.

The other thing on my mind is consistency. If we did this for one foreign-key field on a journal-specific object, I think we'd want to do it for all of them, but that's not easy because not all journal-specific objects have a simple relation to a journal. We could try, by applying a custom form field to a base class that is applied for objects directly related to journal as well as ones tenuously related through another object.

In terms of user experience, if this were an end-user interface where the action would be routine and repetitive, I'd be much more willing to filter the license silently for the user, because it avoids a click and helps them choose the appropriate object for a data model they don't know the details of.

However, I think what we want in admin is comprehensive information and maximum control, since it is staff-facing, or ideally only administrator-facing. So, here I'd prioritise consistency, visibility of status, and control over click minimisation and automatic filtering.

What do you think?

I think the main issue here is that the rule determining which licenses can be selected by each article is not defined in the database schema (i.e. the models). If we were to define this rule on the model, both django forms and admin area would follow it and wouldn't require re-implementation here, thus making it more DRY. We can use pass a callable to the limit_choices_to property on the ForeignKey field set on the model so that only valid licenses can be ever linked to an ariticle:

see https://docs.djangoproject.com/en/1.11/ref/models/fields/#django.db.models.ForeignKey.limit_choices_to

Sadly, this won't have any effect on the database schema as it is only implented at application level.

@joemull if you need help configuring this I'm happy to assist

@joemull
Copy link
Member

joemull commented Mar 1, 2023

@mauromsl I think that's a good implementation. Moving this to 1.6 if that's alright with you as well @ajrbyers.

@joemull joemull removed this from v1.5 Torres Mar 1, 2023
@joemull joemull added this to Old v1.6 Mar 1, 2023
@github-project-automation github-project-automation bot moved this to Todo in Old v1.6 Mar 1, 2023
@joemull joemull removed their assignment Mar 1, 2023
@joemull joemull changed the title Limit License Queryset in Admin Limit the licenses that can be linked to an article, by journal Mar 1, 2023
@ajrbyers
Copy link
Member Author

ajrbyers commented Mar 1, 2023

@joemull that sounds good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev-ready This issue has been refined and is ready for development. enhancement Add or improve something on an existing feature size M Medium
Projects
No open projects
Status: 📋 Todo
Development

No branches or pull requests

3 participants