Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
Already on GitHub? Sign in to your account
AO3-3704 Users can add coauthor work skins to works #2994
Conversation
| @@ -39,4 +39,47 @@ | ||
| end | ||
| + describe '#all_coauthor_skins' do |
| @@ -215,5 +215,93 @@ | ||
| end | ||
| + describe '.approved_or_owned_by' do |
sarken
added
the
Awaiting review
label
Aug 10, 2017
sarken
approved these changes
Sep 20, 2017
This looks good! Just two very minor comments about formatting the tests.
| + And I coauthored the work "Shared" as "lead_author" with "coauthor" | ||
| + And I am logged in as "lead_author" | ||
| + When I edit the work "Shared" | ||
| + Then I should see "Lead Author's Work Skin" within "#work_work_skin_id" |
sarken
Sep 20, 2017
Owner
For Cucumber tests, we generally prefer to use the label text rather than the field id or name to better reflect what the user sees.
cyrilcee
Sep 27, 2017
Contributor
This ask would be pretty straightforward if I was looking at the actual value of the field, but testing what select options are available (not selected) with only a given label is surprisingly complicated.
I updated line 159 to use the label, since I'm just selecting in that line. But for lines 156-158, how important is it to create a new step that lets people test the presence of select options by label?
sarken
Sep 27, 2017
Owner
Oh, sorry about that! I thought The field labeled "Label" should contain "option" would work do that, but it looks like it can't handle select fields with more than one option. I think we can live with the field id here.
| + | login | | ||
| + | lead_author | | ||
| + | coauthor | | ||
| + And I am logged in as "lead_author" |
sarken
Sep 20, 2017
Owner
Could you indent the Ands two spaces deeper than the Givens, Whens, and Thens?
sarken
added
Reviewed: Needs Coder Action
and removed
Awaiting review
labels
Sep 21, 2017
sarken
approved these changes
Sep 27, 2017
Yay, looks good! Thank you, and sorry about the confusion with the select field. Once Travis is done running, we can get this merged
Just one quick note for the future -- while you're definitely welcome to squash commits or otherwise rewrite a branch's history before opening a pull request, once a pull request is open, we prefer folks to just push any additional changes without squashing. We squash all commits on merge to keep things nice and tidy, and squashing them on a branch with an open pull request can make things a little harder to review, since we lose links to previous Travis runs and often can't use GitHub's View Changes function.
cyrilcee commentedAug 9, 2017
Issue
https://otwarchive.atlassian.net/browse/AO3-3704
Purpose
Users can now add coauthors' work skins to works from the Edit Work page. Users still won't be able to add coauthors' work skins from the Edit Multiple Works page.
Testing
If bugfix is working, co-author should be able to edit work without original author's work skin getting stripped. They should also be able to add or remove their coauthor's work skin at any time.