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.

Sign up for GitHub

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

Jump to bottom

[DRUP-624] Add revisions to API Docs. #3

Merged
merged 17 commits into from
May 26, 2019
Merged

[DRUP-624] Add revisions to API Docs. #3

merged 17 commits into from
May 26, 2019

Conversation

Jaesin
Copy link
Contributor

@Jaesin Jaesin commented May 17, 2019

@googlebot
Copy link

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of all the commit author(s), set the cla label to yes (if enabled on your project), and then merge this pull request when appropriate.

ℹ️ Googlers: Go here for more info.

@cnovak cnovak added the cla: yes Indicates CLA has been signed label May 18, 2019
@cnovak cnovak added this to the 1.0.0-beta1 milestone May 22, 2019
@Jaesin
Copy link
Contributor Author

Jaesin commented May 22, 2019

Test results after a rebase:

PHPUnit 6.5.13 by Sebastian Bergmann and contributors.

Testing .../modules/contrib/apigee_api_catalog/tests


Time: 59.27 seconds, Memory: 6.00MB

OK (14 tests, 158 assertions)

Jaesin
Jaesin commented May 23, 2019
View reviewed changes
Copy link
Contributor Author

@Jaesin Jaesin left a comment

Choose a reason for hiding this comment

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

I think this PR is in pretty good shape. The only thing I see is missing is support for deleting a revision like is available with nodes.

src/Entity/Access/ApiDocAccessControlHandler.php Outdated Show resolved Hide resolved
src/Entity/Access/ApiDocAccessControlHandler.php Outdated Show resolved Hide resolved
@Jaesin Jaesin mentioned this pull request May 24, 2019
Closed
cnovak
cnovak approved these changes May 25, 2019
View reviewed changes
Copy link
Collaborator

@cnovak cnovak left a comment

Choose a reason for hiding this comment

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

Approved, but would recommend changing View API Docs to View API Doc as indicated in review

apigee_api_catalog.permissions.yml Outdated Show resolved Hide resolved
apigee_api_catalog.permissions.yml Outdated Show resolved Hide resolved
src/Entity/ApiDoc.php Outdated Show resolved Hide resolved
src/Entity/ApiDoc.php
// If no revision author has been set explicitly, make the current user
// the revision author.
if (!$this->getRevisionUser()) {
$this->setRevisionUserId(\Drupal::currentUser()->id());
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe you said on an earlier comment this is how node entities work. I would expect if there is not an author the author should be anonymous user, but wonder this is for new entities that are not yes saved.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we should look at this in a follow-up. @arlina-espinoza Do you remember why this had to be added?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Added issue #18 for this

src/Entity/ApiDoc.php Outdated Show resolved Hide resolved
src/Entity/ApiDoc.php Show resolved Hide resolved
@cnovak cnovak mentioned this pull request May 25, 2019
Closed
@cnovak cnovak merged commit 3b82466 into apigee:8.x-1.x May 26, 2019
@Jaesin Jaesin deleted the 8.x-1.x-DRUP-624 branch May 28, 2019 22:01
@pradeep-developer pradeep-developer mentioned this pull request Mar 28, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cnovak cnovak cnovak approved these changes

Assignees
No one assigned
Labels
cla: yes Indicates CLA has been signed
Projects
None yet
Milestone
1.0.0-beta1
Development

Successfully merging this pull request may close these issues.
4 participants
@Jaesin @googlebot @cnovak @arlina-espinoza