Esri / arcgis-rest-js

compact, modular JavaScript wrappers for the ArcGIS REST API
https://developers.arcgis.com/arcgis-rest-js/
Apache License 2.0
353 stars 121 forks source link

Developer Credential Package Initial Phase #1111

Closed yida-tong closed 1 year ago

yida-tong commented 1 year ago

Please review:

  1. API Key Management logic. (Retrieve, Create, Update)
  2. App Management logic. (Retrieve, Create)
  3. Documentation contents.

Test cases are fully covered. If I miss some test aspects, let me know too.

yida-tong commented 1 year ago

@noahmulfinger thanks for your review! Changes have been made.

yida-tong commented 1 year ago

@gr-maps thanks for your inputs!

  1. Demo README added. Please review at your convenience.

  2. About second point, have you tried npm run build at root level? Seem like missing @esri/arcgis-rest-developer-credentials/ in node_modules folder. Let me know feedback.

  3. About third point, cdn script/stylesheet doesn't occupy package size in my opinion. I would prefer to put that task later if it is not too urgent. I am open to discuss.

yida-tong commented 1 year ago

@patrickarlt OAuth app task done.

Branch coverage can't reach to 100% even though all test cases are carefully designed.

Screenshot at Jul 20 13-54-10

Add @noahmulfinger @gr-maps @all-reviewers, please re-review at your convenience. Thanks!

codecov[bot] commented 1 year ago

Codecov Report

Merging #1111 (cf77679) into main (170c450) will not change coverage. Report is 1 commits behind head on main. The diff coverage is 100.00%.

:exclamation: Current head cf77679 differs from pull request most recent head 94e055d. Consider uploading reports for the commit 94e055d to get more accurate results

@@            Coverage Diff             @@
##              main     #1111    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files          163       177    +14     
  Lines         3003      3239   +236     
  Branches       534       549    +15     
==========================================
+ Hits          3003      3239   +236     
Files Changed Coverage Δ
packages/arcgis-rest-portal/src/items/create.ts 100.00% <ø> (ø)
...gis-rest-developer-credentials/src/createApiKey.ts 100.00% <100.00%> (ø)
...s-rest-developer-credentials/src/createOAuthApp.ts 100.00% <100.00%> (ø)
...gis-rest-developer-credentials/src/deleteApiKey.ts 100.00% <100.00%> (ø)
...s-rest-developer-credentials/src/deleteOAuthApp.ts 100.00% <100.00%> (ø)
...arcgis-rest-developer-credentials/src/getApiKey.ts 100.00% <100.00%> (ø)
...-rest-developer-credentials/src/getOAuthAppInfo.ts 100.00% <100.00%> (ø)
...ges/arcgis-rest-developer-credentials/src/index.ts 100.00% <100.00%> (ø)
...developer-credentials/src/shared/enum/PRIVILEGE.ts 100.00% <100.00%> (ø)
...per-credentials/src/shared/getRegisteredAppInfo.ts 100.00% <100.00%> (ø)
... and 5 more