Add migration documents from ASM #143

tashirosota · 2021-07-02T12:28:03Z

tashirosota · 2021-07-02T13:40:27Z

@okuramasafumi
Hi. I made a template for the time being.
Let me check some,

 - Is the location correct?
 - How pattern do you want migration case?
 - Is there anything you would like to mention in particular?

okuramasafumi · 2021-07-03T13:42:00Z

Is the location correct?

doc directory is fine, but this directory is also used by yard (that's why it's ignored).
We could use docs directory so that we can separate handwritten documents with autogenerated documents.

Also I want to mention that file name should be something like "migrate_from_jbuilder", not just "jbuilder".

How pattern do you want migration case?

I think we need migration document for real-world, complex cases. For example, with jbuilder we use partials a lot so I'd like to mention how to write partial-like serializer with Alba.

Is there anything you would like to mention in particular?

It's important to mention that Alba doesn't have all the features AMS or jbuilder have. For example, Alba doesn't support cache. I think it's fair to mention some feature Alba doesn't have.

tashirosota · 2021-07-05T01:12:38Z

@okuramasafumi
Thanks for review！I'll fix in this week.

tashirosota · 2021-07-12T13:50:28Z

@okuramasafumi
I'm sorry it will take longer than expected!
If I'm in a hurry, I'd like to divide the PR for each serializer, but what about?
I will start with the ActiveModelSerializers that we use all the time.

okuramasafumi · 2021-07-13T06:27:03Z

@tashirosota Yeah just adding a document for AMS is fine.

tashirosota · 2021-07-13T12:11:11Z

@okuramasafumi
Hi okura.Please review for this！
~~I usually use ASM, couldn't find unsupported ASM's features, if you know then, teach me.~~
~~And please review for pr.~~

okuramasafumi · 2021-07-25T12:33:25Z

docs/migrate_from_active_model_serializers.md

+
+This guide is aimed at helping ActiveModelSerializers users transition to Alba, and it consists of three parts:
+
+1. Basically serialization


Suggested change

1. Basically serialization

1. Basic serialization

okuramasafumi · 2021-07-25T12:33:39Z

docs/migrate_from_active_model_serializers.md

+This guide is aimed at helping ActiveModelSerializers users transition to Alba, and it consists of three parts:
+
+1. Basically serialization
+2. Complexity serialization


Suggested change

2. Complexity serialization

2. Complex serialization

okuramasafumi · 2021-07-25T12:34:09Z

docs/migrate_from_active_model_serializers.md

+end
+
+class Article
+  attr_accessor :id, :title, :body


Suggested change

attr_accessor :id, :title, :body

attr_accessor :user_id, :title, :body

okuramasafumi · 2021-07-25T12:34:25Z

docs/migrate_from_active_model_serializers.md

+end
+```
+
+## 1. Basically serialization


Suggested change

## 1. Basically serialization

## 1. Basic serialization

okuramasafumi · 2021-07-25T12:35:43Z

docs/migrate_from_active_model_serializers.md

+=> {
+  user: {
+    id: id,
+    created_at: created_at,
+    updated_at: updated_at
+  }
+}


Suggested change

=> {

user: {

id: id,

created_at: created_at,

updated_at: updated_at

}

}

# => {

# user: {

# id: id,

# created_at: created_at,

# updated_at: updated_at

# }

# }

When we print output, it should be commented out to make it clear that it's no actually code

okuramasafumi · 2021-07-25T12:59:49Z

docs/migrate_from_active_model_serializers.md

+}
+```
+
+#### When Alba


Suggested change

#### When Alba

#### Alba

okuramasafumi · 2021-07-25T13:00:15Z

docs/migrate_from_active_model_serializers.md

+=> {
+  email: email
+}


Suggested change

=> {

email: email

}

# => {

# email: email

# }

okuramasafumi · 2021-07-25T13:00:37Z

docs/migrate_from_active_model_serializers.md

+}
+```
+
+#### When Alba


Suggested change

#### When Alba

#### Alba

okuramasafumi · 2021-07-25T13:01:26Z

docs/migrate_from_active_model_serializers.md

+=> {
+  :id=>1,
+  :created_at=>created_at,
+  :updated_at=>updated_at,
+  :custom_params=>{
+    :given_params=>{
+      :a=>:b
+    }
+  }
+}


Suggested change

=> {

:id=>1,

:created_at=>created_at,

:updated_at=>updated_at,

:custom_params=>{

:given_params=>{

:a=>:b

}

}

}

# => {

# :id=>1,

# :created_at=>created_at,

# :updated_at=>updated_at,

# :custom_params=>{

# :given_params=>{

# :a=>:b

# }

# }

# }

okuramasafumi · 2021-07-25T13:01:50Z

docs/migrate_from_active_model_serializers.md

+=> {
+  :id=>1,
+  :created_at=>created_at,
+  :updated_at=>updated_at,
+  :custom_params=>{
+    :a=>:b
+  }
+}


Suggested change

=> {

:id=>1,

:created_at=>created_at,

:updated_at=>updated_at,

:custom_params=>{

:a=>:b

}

}

# => {

# :id=>1,

# :created_at=>created_at,

# :updated_at=>updated_at,

# :custom_params=>{

# :a=>:b

# }

# }

okuramasafumi · 2021-07-25T13:10:02Z

@tashirosota I suggested multiple changes including fixing typos and commenting out outputs.

https://github.com/rails-api/active_model_serializers/blob/0-10-stable/docs/howto/serialize_poro.md
This document shows that serializing PORO with AMS is pretty hard. Should we use ActiveModel objects for examples instead?

tashirosota · 2021-07-29T14:14:02Z

docs/migrate_from_active_model_serializers.md

+Example clsss is included `ActiveModel::Model`, because [serializing PORO with AMS is pretty hard](https://github.com/rails-api/active_model_serializers/blob/0-10-stable/docs/howto/serialize_poro.md).This example also works with ActiveRecord.
+
+```rb
+class User


@okuramasafumi
Thanks for review！
I can't find how to serialize with object included ActiveModel::Model.
https://github.com/rails-api/active_model_serializers/tree/0-10-stable#high-level-behavior
SomeResource < ActiveModelSerializers::Model is a bad example of both compatibility, so I'd like to use SomeResource < ActiveRecord::Base, but is that okay?
Like this.

class User < ActiveRecord::Base # columns: id, created_at, updated_at has_one :profile has_many :articles end

Sorry for the late reply.
Yes, using AR::Base is fine.

codecov · 2021-08-21T12:19:29Z

Codecov Report

Merging #143 (1aec442) into main (755a9c6) will increase coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #143      +/-   ##
==========================================
+ Coverage   99.37%   99.40%   +0.02%     
==========================================
  Files           9        9              
  Lines         322      336      +14     
==========================================
+ Hits          320      334      +14     
  Misses          2        2

Impacted Files	Coverage Δ
lib/alba.rb	`100.00% <0.00%> (ø)`
lib/alba/resource.rb	`99.39% <0.00%> (+0.03%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 755a9c6...1aec442. Read the comment docs.

okuramasafumi

Thank you!

tashirosota force-pushed the issues/98 branch 3 times, most recently from e932206 to 7ffbf3a Compare July 2, 2021 13:35

tashirosota force-pushed the issues/98 branch from 7ffbf3a to c5f71f8 Compare July 12, 2021 13:46

tashirosota force-pushed the issues/98 branch from c5f71f8 to dab606f Compare July 12, 2021 15:16

tashirosota force-pushed the issues/98 branch from dab606f to 5bf4b4c Compare July 13, 2021 12:06

tashirosota changed the title ~~[WIP] Add migration documents~~ Add migration documents Jul 13, 2021

tashirosota marked this pull request as ready for review July 13, 2021 12:07

tashirosota force-pushed the issues/98 branch from 5bf4b4c to 8048495 Compare July 13, 2021 12:13

tashirosota changed the title ~~Add migration documents~~ Add migration documents from ASM Jul 13, 2021

tashirosota force-pushed the issues/98 branch 4 times, most recently from b3bb901 to 81dc546 Compare July 13, 2021 12:33

tashirosota force-pushed the issues/98 branch from 81dc546 to 2c978c5 Compare July 20, 2021 13:45

okuramasafumi reviewed Jul 25, 2021

View reviewed changes

tashirosota force-pushed the issues/98 branch 2 times, most recently from 0b1369a to 0eeb5d2 Compare July 29, 2021 13:58

tashirosota commented Jul 29, 2021

View reviewed changes

tashirosota force-pushed the issues/98 branch 4 times, most recently from c291bf9 to 8625ebc Compare August 3, 2021 13:01

Migrate from active_model_serializers

1aec442

tashirosota force-pushed the issues/98 branch from 8625ebc to 1aec442 Compare August 3, 2021 13:01

okuramasafumi approved these changes Aug 21, 2021

View reviewed changes

okuramasafumi merged commit f91b492 into okuramasafumi:main Aug 21, 2021

okuramasafumi mentioned this pull request Oct 30, 2021

Add migration documents #98

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add migration documents from ASM #143

Add migration documents from ASM #143

tashirosota commented Jul 2, 2021

tashirosota commented Jul 2, 2021

okuramasafumi commented Jul 3, 2021

tashirosota commented Jul 5, 2021

tashirosota commented Jul 12, 2021

okuramasafumi commented Jul 13, 2021

tashirosota commented Jul 13, 2021 •

edited

Loading

okuramasafumi Jul 25, 2021

okuramasafumi Jul 25, 2021

okuramasafumi Jul 25, 2021

okuramasafumi Jul 25, 2021

okuramasafumi Jul 25, 2021

okuramasafumi Jul 25, 2021

okuramasafumi Jul 25, 2021

okuramasafumi Jul 25, 2021

okuramasafumi Jul 25, 2021

okuramasafumi Jul 25, 2021

okuramasafumi commented Jul 25, 2021

tashirosota Jul 29, 2021 •

edited

Loading

okuramasafumi Aug 21, 2021

codecov bot commented Aug 21, 2021 •

edited

Loading

okuramasafumi left a comment


		This guide is aimed at helping ActiveModelSerializers users transition to Alba, and it consists of three parts:

		1. Basically serialization

	attr_accessor :id, :title, :body
	attr_accessor :user_id, :title, :body

-=> {
-  user: {
-    id: id,
-    created_at: created_at,
-    updated_at: updated_at
-  }
-}
+# => {
+#   user: {
+#     id: id,
+#     created_at: created_at,
+#     updated_at: updated_at
+#   }
+# }

-=> {
-  :id=>1,
-  :created_at=>created_at,
-  :updated_at=>updated_at,
-  :custom_params=>{
-    :given_params=>{
-      :a=>:b
-    }
-  }
-}
+# => {
+#   :id=>1,
+#   :created_at=>created_at,
+#   :updated_at=>updated_at,
+#   :custom_params=>{
+#     :given_params=>{
+#       :a=>:b
+#     }
+#   }
+# }

Add migration documents from ASM #143

Add migration documents from ASM #143

Conversation

tashirosota commented Jul 2, 2021

tashirosota commented Jul 2, 2021

okuramasafumi commented Jul 3, 2021

tashirosota commented Jul 5, 2021

tashirosota commented Jul 12, 2021

okuramasafumi commented Jul 13, 2021

tashirosota commented Jul 13, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

okuramasafumi commented Jul 25, 2021

tashirosota Jul 29, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Aug 21, 2021 • edited Loading

Codecov Report

okuramasafumi left a comment

Choose a reason for hiding this comment

tashirosota commented Jul 13, 2021 •

edited

Loading

tashirosota Jul 29, 2021 •

edited

Loading

codecov bot commented Aug 21, 2021 •

edited

Loading