feat(docs-infra): add stable to the list of api statuses #34981
Conversation
gkalpak
added
state: WIP
target: patch
There was a problem hiding this comment.
Thx, @ajitsinghkaler! Can you also add some tests for the new feature?
| @@ -117,7 +118,7 @@ export class ApiListComponent implements OnInit { | |||
| const matchesQuery = (item: ApiItem) => | |||
| sectionNameMatches || item.name.indexOf(query!) !== -1; | |||
| const matchesStatus = (item: ApiItem) => | |||
| status === 'all' || status === item.stability || (status === 'security-risk' && item.securityRisk); | |||
| status === 'all' || status === item.stability || (status === 'security-risk' && item.securityRisk) || (status === item.stability && !item.securityRisk); | |||
There was a problem hiding this comment.
The last condition seems superfluous, since the earlier status === item.stability would have matched already.
There was a problem hiding this comment.
I added the last one so that the security risk ones does not mix with the stable ones but you are right this condition is not right.
@gkalpak need your views on this should we add security risk items to the stable filter
There was a problem hiding this comment.
@gkalpak and tests have already been added on line 82-85 it was just that the menu item needed to be added.
If we remove security risk from the stable API then the tests need to be updated otherwise these seem enough to me please have a look at the tests and tell me if any other tests are needed
There was a problem hiding this comment.
Yes, security risk is orthogonal to stability, so stable should include securityRisk items as well. I.e. the condition should remain: status === 'all' || status === item.stability || (status === 'security-risk' && item.securityRisk)
Fair enough about the tests. Ideally we should have e2e tests covering this feature, but we don't 😅.
(If you want to add some e2e tests, I won't stop you, but it isn't required for this PR 😁)
|
You can preview f013be0 at https://pr34981-f013be0.ngbuilds.io/. |
Earlier api can be filtered based on security risk and deprecated now added stable as a status for better user experience Fixes angular#30396
ajitsinghkaler
force-pushed
the
api-status-stable
branch
from
f013be0
to
722132c
Compare
|
@gkalpak made the changes I'll add e2e tests later in the weekend will make a new issue for keeping the progress status on it |
|
You can preview 722132c at https://pr34981-722132c.ngbuilds.io/. |
gkalpak
added
action: merge
Earlier api can be filtered based on security risk and deprecated now added stable as a status for better user experience Fixes angular#30396 PR Close angular#34981
Earlier api can be filtered based on security risk and deprecated now added stable as a status for better user experience Fixes angular#30396 PR Close angular#34981
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Earlier api can be filtered based on security risk and deprecated now added stable as a status for better user experience
Fixes #30396
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Earlier only api can be filtered based on deprecated and security
Issue Number: #30396
What is the new behavior?
Api can be filtered using stable also
Does this PR introduce a breaking change?
Other information