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

Shared authorship #890

Open
wants to merge 8 commits into
base: dev
Choose a base branch
from
Open

Shared authorship #890

wants to merge 8 commits into from

Conversation

lbulgarelli
Copy link
Member

This adds a new shared authorship functionality when submitting projects.

  • BaseAuthor and PublishedAuthor now have a shared field, which stores a number corresponding to a group of authors that contributed equally
  • Logic for swapping authorship position was changed to account for shared authorship groups

@lbulgarelli lbulgarelli force-pushed the authors branch 5 times, most recently from f8f7c89 to 36dcef1 Compare February 25, 2020 01:47
@bemoody
Copy link
Collaborator

bemoody commented Feb 27, 2020

Thank you for working on this, Lucas. There are still some (mostly minor) issues here, and I realize you may not have any time to work on this further before you leave, but this is a good start and I hope we can get it integrated soon.

I'm following you, more or less, up to commit 2c0aa36.

After that... commit 4b0419f is mostly a stylistic change that seems unrelated to the rest of this series. There are some issues (PublishedAuthor.name is wrong; there's something funky going on with text_affiliations) but in terms of style it's a definite improvement.

Then we come to 36dcef1, and I have no idea what's going on and it seems to be broken. :P What is the intention here, and what is the logic?

@lbulgarelli
Copy link
Member Author

After that... commit 4b0419f is mostly a stylistic change that seems unrelated to the rest of this series. There are some issues (PublishedAuthor.name is wrong; there's something funky going on with text_affiliations) but in terms of style it's a definite improvement.

Although this seems like just style changes, it was necessary I could use annotate. set_display_info sets attributes to the object, and those were overwritten after the annotate call, causing the information to be lost. This fixes that problem and also makes the code a bit cleaner :)

I made it so the properties would have the exact same value as was previously assigned set_display_info, to not unknowingly change something somewhere in the website, so idk a lot about name and text_affiliations. Did I change its behavior somehow or was it wrong before? Anyway, I can change it here if desired or open an issue.

Then we come to 36dcef1, and I have no idea what's going on and it seems to be broken. :P What is the intention here, and what is the logic?

So, in the first implementation, if an author was first in the group and I clicked his up button, the whole group would be moved up, changing position with the previous group/user. That was a bit confusing for the user, so instead, I introduced this change, that removes the up/down function for the first/last authors of a group. It should still be possible to move authors within their groups, which was not working but my last commit should've fixed it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants