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

[RFC] feat(docs): dec milestone #4144

Closed
wants to merge 4 commits into from
Closed

[RFC] feat(docs): dec milestone #4144

wants to merge 4 commits into from

Conversation

dhmlau
Copy link
Member

@dhmlau dhmlau commented Nov 18, 2019

Do not merge this PR

This PR is to propose the Dec milestone. Once we agree, a github issue will be created based on the content. This PR is not meant to be merged.

Currently we've committed in 37 points. It's lower than our team velocity, but it might be more than enough because of the holidays.

Checklist

👉 Read and sign the CLA (Contributor License Agreement) 👈

  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • API Documentation in code was updated
  • Documentation in /docs/site was updated
  • Affected artifact templates in packages/cli were updated
  • Affected example projects in examples/* were updated

👉 Check out how to submit a PR 👈


- [ ] Authentication & Authorization
https://github.com/strongloop/loopback-next/issues/3902
- [ ] [5]The First Scenario: Authenticated orders (a minimal authentication)
Copy link
Member Author

Choose a reason for hiding this comment

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

@raymondfeng @bajtos , we might've talked about this before, I couldn't recall what's left for this task to do.

Copy link
Member

Choose a reason for hiding this comment

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

This story was created about a year ago as a kind of a workaround allowing us to show authentication in practice, without any authorization layer (that was not available at that time). I think the proposal still makes sense from the point of DX offered by shopping example REST API, but then I don't know what's the current status of the example app and what changes were implemented since the issue was created & discussed for the last time.

To be honest, I no longer care about this particular story. If the rest of the team thinks the proposed changes are not worth the effort, then I am fine to close the story as being no longer relevant.

Copy link
Member Author

Choose a reason for hiding this comment

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

@jannyHou @emonddr, what's your thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with @bajtos . We can drop the authenticated orders item, and simply ensure whatever remaining authentication and authorization we want in the shopping example is completed. (right now only a few endpoints have authentication or authorization, and we should discuss if we want more...probably when we discuss what other bells and whistles we want to see in shopping cart example)

Copy link
Contributor

Choose a reason for hiding this comment

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

After Deepak landed his authorization PR, the user/{userId}/orders endpoints are secured with authorization, which means we now compare the userId from path and the one decoded from access token to ensure they match.
If we change the UX to user/orders(which retrieves the user id from token directly) then we need to create new examples for authorization scenarios...

I didn't consider the UX change in #1998 when create the authorization PoC(sorry for that...), but since the authorization is already added in the shopping example, I would rather we close #1998 since it doesn't fit the current design.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for your input. I'll close #1998 then. thanks.

- `Add Support for Partitioned Database`
- `Inclusion of related models`
- `Production deployment`

Copy link
Member

Choose a reason for hiding this comment

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

I'd also like use to complete spike on migrating auth & auth from LB3 to LB4, so that we can start planning work on the migration guide and/or tooling for 2020Q1

docs/_milestone_.md Show resolved Hide resolved

- [ ] [3]Spike: Migration guide from LB3 - Authentication & authorization
https://github.com/strongloop/loopback-next/issues/3719
- [ ] [3]How to migrate user-defined model methods #3949
Copy link
Member

Choose a reason for hiding this comment

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

Personally, I would choose the following order of tasks, but I guess the order does not really matter.

docs/_milestone_.md Show resolved Hide resolved

- [ ] Authentication & Authorization
https://github.com/strongloop/loopback-next/issues/3902
- [ ] [5]The First Scenario: Authenticated orders (a minimal authentication)
Copy link
Member

Choose a reason for hiding this comment

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

This story was created about a year ago as a kind of a workaround allowing us to show authentication in practice, without any authorization layer (that was not available at that time). I think the proposal still makes sense from the point of DX offered by shopping example REST API, but then I don't know what's the current status of the example app and what changes were implemented since the issue was created & discussed for the last time.

To be honest, I no longer care about this particular story. If the rest of the team thinks the proposed changes are not worth the effort, then I am fine to close the story as being no longer relevant.

https://github.com/strongloop/loopback-next/issues/1352

- [ ] [5]Reject create/update requests when data contains navigational
properties https://github.com/strongloop/loopback-next/issues/3439
Copy link
Contributor

Choose a reason for hiding this comment

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

@dhmlau hopefully story #3439 will be completed in this month. It's a committed story and Agnes is actively working on it, I can take it over if we need one more week(still within Nov.).

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks. The milestone plan here is tentative, will depend on what's left from Nov.


- [ ] [5]Contribute OpenAPI spec pieces from extensions
https://github.com/strongloop/loopback-next/issues/3854
- [ ] [3]Add user profile factory for authentication modules
Copy link
Contributor

Choose a reason for hiding this comment

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

I would say #3846 is not a high priority.
"support token based authentication in API Explorer in shopping example" from the epic story is much more important, I don't remember if we have a story for it(probably not), I remember Raymond wants to have a default auth dialog for token based authentication, and I agree with him.

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought #3854 would allow that. no?

Copy link
Contributor

Choose a reason for hiding this comment

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

@dhmlau #3854 is "allow extension contributes spec", "support token based authentication in API Explorer in shopping example" means our framework provide built-in spec

Copy link
Member Author

Choose a reason for hiding this comment

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

yes


- [ ] Authentication & Authorization
https://github.com/strongloop/loopback-next/issues/3902
- [ ] [5]The First Scenario: Authenticated orders (a minimal authentication)
Copy link
Contributor

Choose a reason for hiding this comment

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

After Deepak landed his authorization PR, the user/{userId}/orders endpoints are secured with authorization, which means we now compare the userId from path and the one decoded from access token to ensure they match.
If we change the UX to user/orders(which retrieves the user id from token directly) then we need to create new examples for authorization scenarios...

I didn't consider the UX change in #1998 when create the authorization PoC(sorry for that...), but since the authorization is already added in the shopping example, I would rather we close #1998 since it doesn't fit the current design.

@dhmlau
Copy link
Member Author

dhmlau commented Nov 20, 2019

I've updated the milestone based on the review comments. We now target for 37 points because we need to account for the vacation time. Besides issues we might complete before Dec, if everyone agrees, we'll use this as the Dec milestone. Thanks!

- [ ] Inclusion of related models [MVP]
https://github.com/strongloop/loopback-next/issues/1352

- [ ] [5]Reject create/update requests when data contains navigational
Copy link
Member Author

Choose a reason for hiding this comment

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

If this task is done in Nov, please replace this with

Include related models with a custom scope #3453

- [ ] [3]Add user profile factory for authentication modules
https://github.com/strongloop/loopback-next/issues/3846

- [ ] Add Support for Partitioned Database
Copy link
Contributor

Choose a reason for hiding this comment

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

this is already done :)

Copy link
Member Author

Choose a reason for hiding this comment

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

the blog post?

@dhmlau
Copy link
Member Author

dhmlau commented Dec 3, 2019

Thanks @agnes512 for creating the milestone issue. See #4236.

@dhmlau dhmlau closed this Dec 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants