feat: expand controller action coverage, with specs, and add auto-generated models by dukenguyenxyz · Pull Request #12 · PlaceOS/crystal-client

dukenguyenxyz · 2020-11-10T22:58:53Z

Changes:

Expand coverage to all existing rest-api routes and controller actions, accompanied by specs
Add some generic modules for frequently used methods
Remove all predefined models from placeos-models with autogenerated model macro

…odules to correspond to all rest-api controller actions

… ref, comment where fields are missing, or do not exist in original model

…troller actions and client actions

…s and not body for certain actions

feat: expand controller action coverage, with specs

…s, oauth, repos, settings

…y::Type

Gab tests

…el autogeneration

caspiano

Also noticed that some .DS_STORE files have made it in

caspiano · 2020-11-11T01:12:42Z

src/placeos/api_wrapper/users.cr

+      nickname : String? = "",
+      email : String? = "",
+      phone : String? = "",
+      country : String? = "",
+      image : String? = "",
+      ui_theme : String? = "light",
+      metadata : String? = "",
+      login_name : String? = "",
+      staff_id : String? = "",
+      first_name : String? = "",
+      last_name : String? = "",
+      building : String? = "",
+      password_digest : String? = "",
+      email_digest : String? = "",
+      card_number : String? = "",
+      deleted : Bool? = false,
+      groups : Array(String)? = [] of String,
+      access_token : String? = "",
+      refresh_token : String? = "",
+      expires_at : Int64? = nil,
+      expires : Bool? = false,
+      password : String? = "",
+      sys_admin : Bool? = false,
+      support : Bool? = false


best to have these as nil defaults, or perhaps even do the Undefined type trick so you don't serialise the nil, which may be acceptable for the attribute and lead to an accidental override

caspiano · 2020-11-11T01:14:23Z

src/placeos/api_wrapper/endpoint.cr

+    module Create(T)
+      def create(**args) : T
+        post base, body: from_args, as: T
+      end
+    end
+
+    module Update(T)
+      def update(id, **args) : T
+        post "#{base}/#{id}", body: from_args, as: T


perhaps we can pair on this?

caspiano · 2020-11-11T01:16:15Z

src/placeos/api_wrapper/endpoint.cr

+    # List or search.
+    #
+    # Results maybe filtered by specifying a query - *q* - to search across
+    # attributes. A small query language is supported within this:
+    #
+    # Operator | Action
+    # -------- | ------
+    # `+`      | Matches both terms
+    # `|`      | Matches either terms
+    # `-`      | Negates a single token
+    # `"`      | Wraps tokens to form a phrase
+    # `(`  `)` | Provides precedence
+    # `~N`     | Specifies edit distance (fuzziness) after a word
+    # `~N`     | Specifies slop amount (deviation) after a phrase
+    #
+    # Up to *limit*  will be returned, with a paging based on *offset*.
+    module Search(T)
+      def search(


Suggested change

# List or search.

#

# Results maybe filtered by specifying a query - *q* - to search across

# attributes. A small query language is supported within this:

#

# Operator | Action

# -------- | ------

# `+` | Matches both terms

# `|` | Matches either terms

# `-` | Negates a single token

# `"` | Wraps tokens to form a phrase

# `(` `)` | Provides precedence

# `~N` | Specifies edit distance (fuzziness) after a word

# `~N` | Specifies slop amount (deviation) after a phrase

#

# Up to *limit* will be returned, with a paging based on *offset*.

module Search(T)

def search(

module Search(T)

# List or search.

#

# Results maybe filtered by specifying a query - *q* - to search across

# attributes. A small query language is supported within this:

#

# Operator | Action

# -------- | ------

# `+` | Matches both terms

# `|` | Matches either terms

# `-` | Negates a single token

# `"` | Wraps tokens to form a phrase

# `(` `)` | Provides precedence

# `~N` | Specifies edit distance (fuzziness) after a word

# `~N` | Specifies slop amount (deviation) after a phrase

#

# Up to *limit* will be returned, with a paging based on *offset*.

def search(

caspiano · 2020-11-11T01:20:59Z

src/placeos/api_wrapper/drivers.cr

+    end
+
+    def compiled(id : String)
+      get "#{base}/#{id}/compiled", as: Driver


this will be either 404 or 200, so just return a bool

caspiano · 2020-11-11T01:22:13Z

src/placeos/api_wrapper/drivers.cr

    def create(
      name : String,
-      role : Role,
+      role : Int32,


would be good to have this using an explicit enum.

try making an alias to the actual enum where you generate the models

caspiano · 2020-11-11T01:23:10Z

src/placeos/api_wrapper/auths/base.cr

@@ -1,5 +1,5 @@
 require "../endpoint"
-require "../../api/models/auths/*"
+require "placeos-models"


why is this required here?

caspiano · 2020-11-11T01:33:58Z

src/placeos/api/models/base.cr

+            {% if opts[:converter] && !FORBID_CONVERTERS.includes?(opts[:converter].resolve.stringify) %}
+              @[JSON::Field(converter: {{opts[:converter]}})]
+            {% end %}
+            property {{name.id}} : {{opts[:klass]}}?


Suggested change

property {{name.id}} : {{opts[:klass]}}?

getter {{name.id}} : {{opts[:klass]}}?

you also have access to the actual type of the attribute here, might be good to use that directly rather than making all getters nillable.

for routes that return extra results, you can make them nillable fields. for example

caspiano · 2020-11-11T01:35:34Z

src/placeos/api/models/base.cr

+        {% if subclasses == PlaceOS::Model::SubModel %}
+          include Timestamps
+        {% end %}


i think this is a false assumption

caspiano · 2020-11-11T01:35:54Z

src/placeos/api/models/base.cr

+          include Timestamps
+        {% end %}
+
+        getter id : String? = nil


the id should be non-nil

GabFitzgerald and others added 30 commits October 26, 2020 12:49

test(drivers): first draft of specs written

9512786

reverted chanages to repository files

533b7e9

style(drivers): crystal tool format

3353c2b

feat: add basic template to extend api_wrapper endpoint classes and m…

5ba475f

…odules to correspond to all rest-api controller actions

refactor: update correct positioning of models with placeos::model as…

980bf26

… ref, comment where fields are missing, or do not exist in original model

chore: add templates for all possible specs which map to rest-api con…

920f893

…troller actions and client actions

style: reposition client methods to allign with rest-api action order

26fd016

fix: cast type for response object correctly, set from_args for param…

9a4c6ef

…s and not body for certain actions

fix: remove inheritance for webhook class

e26c00a

refactor: add id fields to models

dc69c7e

broken: role enum - undefined constant

1dae3b9

fix(driver spec): role enum fixed

c97ff2b

style(role enum): include Client::API::Models allows for Role:{role}

331a2d3

test(driver spec): first attempt to test paginatation?

0ae1ea4

Merge branch 'gab-tests' into test/integration

52fb52a

Update drivers.cr

b9763c1

Merge pull request #9 from PlaceOS/test/integration

cd7d238

feat: expand controller action coverage, with specs

chore(driver spec): removed duplicate files

a60058a

test: page spec, left pending

21b47a4

test(driver spec): compile and recompile

943b713

small changes, nothing meaningful

0e99231

test(oauth applications): specs for all methods

b754fd8

test(repositories): destroy, fetch and search methods tested

6bc7fe0

style: crystal tool format, and some other changes

a4b3125

refactor: add more interfaces and update classes

a871c87

refactor: update missing id params

0c2b6c1

ref: implemented search module into cluster, domains, drivers, module…

c907ada

…s, oauth, repos, settings

style: crystal tool format

e392f8a

style: removed trailing white space

e027f9f

test: settings common methods

a3ca125

dukeraphaelng and others added 16 commits November 2, 2020 19:08

chore: update component method array

6bd4af4

chore: fix repo_type in repositories_spec - PlaceOS::Model::Repositor…

d21a773

…y::Type

fix: mistakes current version

f089e22

fix: models fix

ef63dce

style: crystal tool format

5a11fd3

Merge pull request #11 from PlaceOS/gab-tests

1548263

Gab tests

style: crystal tool format bug fixes

b9ec02e

chore: driver role verbose

60a7a35

chore: add repository type coverter with specs

78e0994

chore: resolve merge conflict with repository

7db8a27

chore: add json_serializable to enum from string

65ef0f1

refactor: string value to int32 value for enum

12a12f5

refactor: update all models, remove to_json from spec, add client mod…

619f837

…el autogeneration

chore: update removing all .to_json from specs

003703d

feat: allow auto-generate model passing all specs

92d9f6a

refactor: replace extra model definition with generic method

1c0d681

dukenguyenxyz requested a review from caspiano November 10, 2020 22:58

caspiano reviewed Nov 11, 2020

View reviewed changes

src/placeos/api/models/base.cr

include Timestamps

{% end %}

getter id : String? = nil

Copy link

Contributor

caspiano Nov 11, 2020

Choose a reason for hiding this comment

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

the id should be non-nil

refactor: update partial create and update methods macro

421751b

caspiano assigned dukenguyenxyz and kimburgess Jul 7, 2021

caspiano added the type: enhancement new feature or request label Jul 7, 2021

jeremyw24 unassigned kimburgess and dukenguyenxyz Oct 11, 2021

Merge branch 'master' into refactor/auto-generate-model

678ae8c

caspiano marked this pull request as draft March 9, 2022 01:12

caspiano added type: maintenance requires maintenance priority: low labels Mar 9, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: expand controller action coverage, with specs, and add auto-generated models#12

feat: expand controller action coverage, with specs, and add auto-generated models#12
dukenguyenxyz wants to merge 86 commits intomasterfrom
refactor/auto-generate-model

dukenguyenxyz commented Nov 10, 2020

Uh oh!

caspiano left a comment

Uh oh!

caspiano Nov 11, 2020

Uh oh!

caspiano Nov 11, 2020

Uh oh!

caspiano Nov 11, 2020

Uh oh!

caspiano Nov 11, 2020

Uh oh!

caspiano Nov 11, 2020

Uh oh!

caspiano Nov 11, 2020

Uh oh!

caspiano Nov 11, 2020

Uh oh!

caspiano Nov 11, 2020

Uh oh!

caspiano Nov 11, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	property {{name.id}} : {{opts[:klass]}}?
	getter {{name.id}} : {{opts[:klass]}}?

Conversation

dukenguyenxyz commented Nov 10, 2020

Uh oh!

caspiano left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants