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

Unstage declined requests #8946

Merged
merged 3 commits into from
Jan 19, 2020

Conversation

DavidKang
Copy link
Contributor

@DavidKang DavidKang commented Jan 14, 2020

When a request is on a declined state, we can't touch the reviews. For that, we reopen it temporally to manage the reviews.

Fixes #8757

TODO:

  • Tests

@DavidKang DavidKang added Frontend Things related to the OBS RoR app staging-workflow Things related to the staging workflow feature labels Jan 14, 2020
@DimStar77
Copy link
Contributor

not sure I'd want that - at least in ADIs I often have a group of stuff relying on each other, and a decline means I need a fixed submission incoming - that should go to the same adi group the original thing was in. In case of 'Project based Stagings (think full KDE Stack, Full Qt update) it's the same thing - one request being declined means the full group is only ready when a fix is incoming for that group

@vpereira
Copy link
Contributor

Hey @DimStar77 any idea how to handle it? Another solution would be to avoid decline a request if it's staged (look the issue that we are trying to fix #8757) but then it wouldn't be in line with the #8941

@DimStar77
Copy link
Contributor

Another solution would be to avoid decline a request if it's staged

Clearly not an option - legal as well as the regular source review must be able to decline a request irrespective of it being staged or not.

@coolo
Copy link
Member

coolo commented Jan 14, 2020

declining and accepting are clearly 2 different beasts. For a declined request, the manual unstage needs to manipulate the reviews. The declining action itself should not touch it

@vpereira
Copy link
Contributor

vpereira commented Jan 15, 2020

Clearly not an option - legal as well as the regular source review must be able to decline a request irrespective of it being staged or not.
@DimStar77

I don't really know how you are doing staging, but I was assuming that you just stage a request after those checks are done (as in a CI pipeline), no?

@coolo
Copy link
Member

coolo commented Jan 15, 2020

https://stephan.kulow.org/mermaid.html happens all in parallel

@DavidKang
Copy link
Contributor Author

DavidKang commented Jan 15, 2020

Well I push a second commit where the reviews are handled during the unstage process. The only thing is that I reopen the request to be able manage the reviews. I reopen the request, because I don't like the option to skip validations/policies on requests and reviews.

@DavidKang
Copy link
Contributor Author

https://stephan.kulow.org/mermaid.html happens all in parallel

thanks

@coolo
Copy link
Member

coolo commented Jan 15, 2020

That will do. Thanks

@DavidKang DavidKang force-pushed the feature-unstaging-declined branch 3 times, most recently from bdaa1a0 to 9cf0cd2 Compare January 15, 2020 13:12
@DavidKang DavidKang added review-app Apply this label if you want a review app started and removed review-app Apply this label if you want a review app started labels Jan 15, 2020
When a request is on a declined state, we can't touch the reviews. For
that, we reopen it temporally to manage the reviews.
login user
bs_request.change_state(newstate: 'declined', user: user.login, comment: 'Fake comment')
delete :destroy, params: { staging_workflow_project: staging_workflow.project.name, format: :xml },
body: "<requests><request id='#{bs_request.number}'/></requests>"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see the "test case" in the feature request. The interesting part is if reopening the request leaves the staging project untouched and the request will appear in backlog.

I expect this to be the case, but it would be good to have this future safe.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coolo, what do you mean with untouched?
I extended the test, reopening a declined request that was unstaged.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

untouched == it stays empty after reopen if it was empty before. Just as your test verifies

Request that was decline can only be reopen by the user that decline it
or you have rights on the target package.

Staging Manager should be able to reopen the requests and change the
reviews to send the declined request to the staging backlog if this
request is reopened.
@DavidKang DavidKang changed the title WIP: Unstage declined requests Unstage declined requests Jan 17, 2020
@vpereira vpereira merged commit 99650df into openSUSE:master Jan 19, 2020
@vpereira vpereira deleted the feature-unstaging-declined branch January 19, 2020 07:55
@coolo
Copy link
Member

coolo commented Jan 20, 2020

By this the 'untracked' state should be impossible to reach.

coolo added a commit to coolo/open-build-service that referenced this pull request Jan 20, 2020
This state should be impossible to reach after
openSUSE#8946
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Frontend Things related to the OBS RoR app staging-workflow Things related to the staging workflow feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unstaging a declined request needs to change reviews
4 participants