Fix relationship links doc #1981

groyoh · 2016-11-20T17:55:38Z

Purpose

The relationship links contained incorrect information that mislead users in using AMS improperly. More precisely, JSONAPI does not allow an attribute to be named links. This documentation was fixed and reworked at certain parts to match the latest API.

Changes

Only documentation.

Related GitHub issues

#1971
#1935

Additional helpful information

I haven't reviewed everything yet but I wanted some feedback to see if I'm going in the right direction with theses changes.

@michael-reeves if you have some time, maybe you could also tell me if this documentation is clearer or not.

mention-bot · 2016-11-20T17:55:39Z

@groyoh, thanks for your PR! By analyzing the history of the files in this pull request, we identified @vasilakisfil, @bacarini and @edwinlunando to be potential reviewers.

michael-reeves · 2016-11-21T03:10:28Z

@groyoh
Just my 2 cents, but the usage described here is very confusing to me.

user = User.new(id: 1, name: "John")
resource = ActiveModelSerializer::SerializableResource.new(user, adapter: :json)
resource.to_json

This code (or something similar, sometimes with meta: ) appears in several spots. But it's not entirely obvious where that code belongs in a Rails app. In these docs, it's just hanging out outside of the Serializer class.

Is it supposed to be inside the model, or controller? It's not entirely obvious. I think being as specific as possible keeps it clearer for newbs like me.

In my case, I put the adapter in a active_model_serializer initialization file.

bf4 · 2016-11-21T04:40:29Z

docs/howto/add_link.md

+}
+```
+
+Note that `ActiveModelSerializers` pagination relies on a collection that has the methods `current_page`, `total_pages`, and `size`, such as are supported by both [Kaminari](https://github.com/amatsuda/kaminari) or [WillPaginate](https://github.com/mislav/will_paginate). If you want to have paginated links your collections but don't want to use neither Kaminari nor WillPaginate, make sure to define these methods.


Could use note on how to opt-out of this AMS behavior

Oh true. I did not think about that. Will update it later today.

bf4 · 2016-11-21T04:41:41Z

docs/howto/handle_pagination.md

+resource.to_json
+```
+
+* For usage of top-level links see [add_links.md#pagination].


link missing ()

bf4 · 2016-11-21T04:48:12Z

@michael-reeves The meaning and usage of ActiveModel::Serializer has changed over time. Whereas it was once the primary object in AMS, it no longer is.

user = User.new(id: 1, name: "John")
resource = ActiveModelSerializer::SerializableResource.new(user, adapter: :json)
resource.to_json

does exactly what it appears to: given a user, you can use AMS to serialize it JSON by using SerializableResource. SerializableResource is the same object used in the controller. The equivalent controller code would be

user = User.new(id: 1, name: "John")
render json: user, adapter: :json

Of course, if the default adapter were JSON, then the adapter option would not be required.

In addition, the code assumes there is a UserSerializer class defined.

I'd love if you could open a new issue or PR helping us improve the docs wherever they may be unclear. (And yes, I agree there is plenty of room for improvement).

bf4 · 2016-11-21T04:48:28Z

@groyoh looks great!

groyoh · 2016-11-21T14:12:27Z

@michael-reeves AMS is not framework specific. Therefore it can be used outside of Rails (in my previous job we did not use it with rails at all). That is why I tried to provide a rails version and a non rails version when I could. That can indeed be confusing but that's the AMS API. Maybe i could specify at the top that examples are given for both rails and non rails. Would that make it more clear? Also i specifed the adapter option because I described many adapter within the same file. Therefore if someone wants to copy paste the code, they do not need extra setup.

michael-reeves · 2016-11-21T17:49:24Z

@groyoh
That's completely fair. I thought about it not being Rails specific, but figured it was worth a mention.

I guess I didn't see the Rails specific piece, other than the pagination stuff. And that was super useful, since I'm about to get into that code, so thanks for that.

And it's very helpful that you had code for different adapters. Thanks.

I do think the example @bf4 put in his comment to me above helped clear it up quite a bit.

So, your call if you want to add something like that.

groyoh · 2016-11-22T22:12:50Z

@bf4 I updated the PR based on your comments

bf4 · 2016-11-27T01:50:55Z

CHANGELOG.md

@@ -31,6 +31,8 @@ Fixes:
 - [#1881](https://github.com/rails-api/active_model_serializers/pull/1881) ActiveModelSerializers::Model correctly works with string keys (@yevhene)

 Misc:
+
+- [#1981](https://github.com/rails-api/active_model_serializers/pull/1981) Improve link and pagination documentation(@groyoh)


s/documentation(@/documentation. (@

bf4 · 2016-11-27T01:52:16Z

docs/README.md

@@ -22,10 +22,10 @@ This is the documentation of ActiveModelSerializers, it's focused on the **0.10.

 ## How to

+- [How to use ActiveModelSerializers outside of rails controllers](howto/outside_controller_use.md)


s/rails/Rails

How ActiveModelSerializers Outside Of Rails Controllers

bf4 · 2016-11-27T01:53:06Z

docs/howto/add_link.md

+
+`ActiveModelSerializers` offers you many ways to add links in your JSON reponse depending on the adapter you are using.
+
+Note that within the following examples, we set the `adapter` option, but that is not needed if the `ActiveModelSerializers.config.adapter` variable is properly set. Also, some examples are given for both Rails and [non-Rails usage](outside_controller_use.md).


is configured with that adapter

bf4 · 2016-11-27T01:53:38Z

docs/howto/add_link.md

+
+```ruby
+class User < ActiveModelSerializers::Model
+  attributes :id, :name


assuming #1984 is merged.. :)

Wasn't the attributes method provided by #1982 ?

bf4 · 2016-11-27T01:54:15Z

docs/howto/add_link.md

+
+The JSON API specification allows the usage of the `links` member for representing links in three cases:
+* [Top level links](http://jsonapi.org/format/#document-top-level)
+* [Resource link](http://jsonapi.org/format/#document-resource-object-links)


s/ link/ links

bf4 · 2016-11-27T01:55:26Z

docs/howto/add_link.md

+* [Resource link](http://jsonapi.org/format/#document-resource-object-links)
+* [Relationship links](http://jsonapi.org/format/#document-resource-object-relationships)
+
+`ActiveModelSerializers` provides tools to handle these links.


ActiveModelSerializers supports the JSON API links specification.

(or complies with) or (meets)

docs/howto/add_link.md

bf4 · 2016-11-27T02:00:21Z

docs/howto/add_link.md

+```ruby
+class UserController < ActionController::Base
+  def kaminari
+    users = User.all.to_a # .all.to_a used as example


I think we can leave this out and just say see those gems for how to use them.

And example could just be, assuming we're rendering the third page, one item per page

render json: users, adapter: :json_api

would render as:

bf4 · 2016-11-27T02:03:20Z

docs/howto/add_link.md

+}
+```
+
+Note that it is possible to opt-out `ActiveModelSerializers` pagination relies on a collection that has the methods `current_page`, `total_pages`, and `size`, such as are supported by both [Kaminari](https://github.com/amatsuda/kaminari) or [WillPaginate](https://github.com/mislav/will_paginate). If you want to have paginated links your collections but don't want to use neither Kaminari nor WillPaginate, make sure to define these methods.


? vs. jsonapi_pagination_links_enabled?

Looks like this was a mix between two different docs. Not sure how that happened.

Should be

-Note that it is possible to opt-out `ActiveModelSerializers` pagination relies on a collection that has the methods `current_page`, `total_pages`, and `size`, such as are supported by both [Kaminari](https://github.com/amatsuda/kaminari) or [WillPaginate](https://github.com/mislav/will_paginate). If you want to have paginated links your collections but don't want to use neither Kaminari nor WillPaginate, make sure to define these methods. +Note that `ActiveModelSerializers` automatic pagination relies on a collection that has the methods `current_page`, `total_pages`, and `size`, such as are supported by both [Kaminari](https://github.com/amatsuda/kaminari) or [WillPaginate](https://github.com/mislav/will_paginate). If you want to have paginated links for your collections but don't want to use neither Kaminari nor WillPaginate, make sure to define these methods.

bf4 · 2016-11-27T02:05:50Z

docs/howto/add_link.md

+  attributes :name
+
+  link(:self) { user_path(object.id) }
+  link(:posts) { posts_path(user_id: object.id) }


Using url helpers *requires*: 1) that the SerializationContext has the url helpers configured. This is done for you when rendering from a controller 2) that a block is passed where the url_helpers are used

Would it make more sense to only gives examples with hand-crafted urls and then add a Using Rails url helpers section?

Or using url_helpers since it's supported outside Rails...

Sure, having the section break down like that makes sense.

bf4 · 2016-11-27T02:06:31Z

docs/howto/add_link.md

+end
+
+user = User.new(id: 1, name: "Kelly")
+resource = ActiveModelSerializers::SerializableResource.new(user, adapter: :json_api)


this won't work without a SerializationContext. We need to verify all our docs contain valid code.

bf4 · 2016-11-27T02:07:58Z

docs/howto/add_link.md

+  attributes :id, :name
+  has_many :posts do
+    link(:self) do
+      user_posts_path(object.id) # Rails url helper automatically available


See https://github.com/rails-api/active_model_serializers/pull/1981/files#r89681745

i.e. has the same api and requirements as resource links, but link is called in the context of an association block.

bf4 · 2016-11-27T02:11:17Z

docs/howto/add_link.md

+
+## JSON adapter
+
+The `json` is not able to handle links defined by the `link` class method. However, you can either use the `meta`  member or define a `links` attribute on the serializer.


JSON adapter doesn't respect the link macro in resources or relationships, however it does respect 'top-level meta', which can be used to include links. See the implementation of the https://github.com/rails-api/active_model_serializers/blob/v0.10.3/lib/active_model_serializers/adapter/json_api/link.rb for more details

bf4 · 2016-11-27T02:11:47Z

docs/howto/add_link.md

+```ruby
+class UserSerializer < ActiveModel::Serializer
+  attributes :id, :name, :links
+  attribute :links do


we really shouldn't allow this.. e.g. https://github.com/cerebris/jsonapi-resources/blob/df5a0f5f938f829eaba67dd79b79b820a7c58b74/lib/jsonapi/resource.rb#L1104-L1109

Why not? If people use JSON or Attributes adapters, they have every right to use a links attribute.

well, that goes back to our design goals of making all serializers/adapters support links

Of course, supporting links make sense. But what if some users have a specific use case where they need to build non JSONAPI or legacy endpoints that would provide a links attributes that has nothing to do with hyperlink (lets say something related to joins, chains, etc. I admit that this might never happen too but I don't think we should forbid such a thing. At least not for non JSONAPI adapters.

bf4 · 2016-11-27T02:13:19Z

docs/howto/add_link.md

+
+```ruby
+class UserSerializer < ActiveModel::Serializer
+  include ActiveModelSerializers::SerializationContext::UrlHelpers


I would emphasize that this is not our recommended usage, and reference include Rails.application.url_helpers like you'd see in other docs.

Why don't we recommend that? What's wrong with doing so?

So, we don't do this because it couples the serializer too tightly to both Rails and to the routes.

We also provide a way to use url helpers via the serialization context.

And depending on how this is written, we're going to get a ton of questions all the time why don't we just mix it in like this?

Also, just feels messy :(

Yeah I get your point. AFAIK serialization context is not provided to links or is it?

bf4 · 2016-11-27T02:14:23Z

docs/howto/add_link.md

+
+## Attributes adapter
+
+The only way to provide pagination with the `attributes` adapter is to define a `links` attribute [as described for the JSON adapter above](#via-an-attribute).


which honestly, is kind of dumb of us. Why do we forbid having a meta attribute? Attributes adapter really needs to be reabsorbed into JSON, since it's really the domain of the serializer.

How would you add a meta to:

[ { "id": 1, "name": "John" }, { "id": 2, "name": "Margaret" } ]

?

I still think Attributes has its own place here.

Good point re: collections, but in terms of 'resource' meta/links...

ActiveModel::Serializers::JSON.include_root_in_json == false -> 'Attributes' adapter

ActiveModel::Serializers::JSON.include_root_in_json == true -> 'JSON' adapter

only difference is presence of root, then what that implies. https://github.com/rails/rails/blob/5-0-stable/activemodel/lib/active_model/serializers/json.rb#L88-L101

def as_json(options = nil) root = if options && options.key?(:root) options[:root] else include_root_in_json end if root root = model_name.element if root == true { root => serializable_hash(options) } else serializable_hash(options) end end

bf4 · 2016-11-27T02:15:08Z

docs/howto/add_relationship_links.md

-relationship data must be under the `related` attribute, whereas to manipulate the
-relationship (in case of many-to-many relationship) must be under the `self` attribute.
-
-You can find more info in the [spec](http://jsonapi.org/format/#document-resource-object-relationships).


why was all this removed?

Because I removed the documentation which described how links can be defined as attribute for the JSONAPI adapter as it is not part of the specs. Therefore this part of the doc is no more necessary in my opinion.

But it has some good info in it, like about ember-data and n+1 queries, no?

Well that's indeed a good note, but I don't feel like this should be part of this documentation. I mean, it's right in the middle of the link documentation. How is anyone suppose to find that out. Maybe I could add another section which discuss the use of include_data false and the how to fix n+1 queries issue.

bf4 · 2016-11-27T02:16:56Z

docs/howto/handle_pagination.md

+
+## JSON API adapter
+
+If you want to paginate a collection with the `json_api` adapter, you can either use the top-level `links` member to provide pagination links or the top-level `meta` member to provide pagination information:


this should be cross-referenced in the links section or in it altogether, no?

I already reference the link documentation below. I don't see why we should put meta in the links documentation though. That's two different things in my opinion.

bf4 · 2016-11-27T02:17:46Z

@groyoh PR is pretty ambitious. Maybe good to break into smaller pieces? Reorganizing vs. rewriting vs. fixing bugs?

groyoh · 2016-11-27T12:22:13Z

docs/howto/add_link.md

+class UserSerializer < ActiveModel::Serializer
+  include ActiveModelSerializers::SerializationContext::UrlHelpers
+
+  attributes :id, :name, :links


- attributes :id, :name, :links + attributes :id, :name

groyoh · 2016-12-30T20:04:47Z

@bf4 I reworked this PR to only include the fix to the documentation about JSONAPI (plus minor typos). I will create a different PR for reorganizing the docs.

bf4 · 2017-01-07T05:37:06Z

docs/howto/add_relationship_links.md


 You can define an attribute in the resource, named `links`.

 ```ruby
 class Api::V1::UserSerializer < ActiveModel::Serializer
-  attributes :id, :name, :links
+  include Rails.application.routes.url_helpers


You can define an attribute in the resource, named links`.

Isn't this true for all the adapters? It's just especially confusing for JSONAPI. Also, the name links here is arbitrary once you're mixing in the url helpers and just making an attribute return an custom object (hash).

I'd also rather encourage usage of the serialization context over mixing in include Rails.application.routes.url_helpers but that's me. I sorta think using the include Rails.application.routes.url_helpers should be a way of doing it until we unify the adapters or write better docs for serialization_context.

Needs tests. That's part of the reason the doc fix is needed, right? we could put that off into a followup pr?

It is working for all the adapter but not correct for JSONAPI as the spec forbid to have an attribute named links:

The value of the attributes key MUST be an object (an “attributes object”). Members of the attributes object (“attributes”) represent information about the resource object in which it’s defined.

Attributes may contain any valid JSON value.

Complex data structures involving JSON objects and arrays are allowed as attribute values. However, any object that constitutes or is contained in an attribute MUST NOT contain a relationships or links member, as those members are reserved by this specification for future use.

Also I had the url helper because within the links method there was a call to api_v1_user_path. And that call would not be correct without the url helper. I simply "fixed" the existing doc as I did not want to modify it to avoid having too many changes.

I suppose they should do it like ActiveModelSerializers::Adapter::JsonApi::Link and include ActiveModelSerializers::SerializationContext::UrlHelpers

include ActiveModelSerializers::SerializationContext::UrlHelpers

@bf4 Well I actually followed your own advices #1981 (comment). 😁 Now who should I listen to? @bf4 from the past or @bf4 from the present? 😕

oh dear god. I guess I... good eyes @groyoh

bf4

If we're going to include url_helpers, let's do it the AMS way

bf4 · 2017-01-07T21:50:53Z

docs/howto/add_relationship_links.md


 You can define an attribute in the resource, named `links`.

 ```ruby
 class Api::V1::UserSerializer < ActiveModel::Serializer
-  attributes :id, :name, :links
+  include Rails.application.routes.url_helpers


I suppose they should do it like ActiveModelSerializers::Adapter::JsonApi::Link and include ActiveModelSerializers::SerializationContext::UrlHelpers

include ActiveModelSerializers::SerializationContext::UrlHelpers

bf4 · 2017-01-10T05:58:09Z

@groyoh Merged it. Is improvement :)

Fix relationship links doc Conflicts: CHANGELOG.md

* Merge pull request #1990 from mxie/mx-result-typo Fix typos and capitalization in Relationship Links docs [ci skip] * Merge pull request #1992 from ojiry/bump_ruby_versions Run tests by Ruby 2.2.6 and 2.3.3 * Merge pull request #1994 from bf4/promote_architecture Promote important architecture description that answers a lot of questions we get Conflicts: docs/ARCHITECTURE.md * Merge pull request #1999 from bf4/typos Fix typos [ci skip] * Merge pull request #2000 from berfarah/patch-1 Link to 0.10.3 tag instead of `master` branch * Merge pull request #2007 from bf4/check_ci Test was failing due to change in JSON exception message when parsing empty string * Swap out KeyTransform for CaseTransform (#1993) * delete KeyTransform, use CaseTransform * added changelog Conflicts: CHANGELOG.md * Merge pull request #2005 from kofronpi/support-ruby-2.4 Update jsonapi runtime dependency to 0.1.1.beta6 * Bump to v0.10.4 * Merge pull request #2018 from rails-api/bump_version Bump to v0.10.4 [ci skip] Conflicts: CHANGELOG.md * Merge pull request #2019 from bf4/fix_method_redefined_warning Fix AMS warnings * Merge pull request #2020 from bf4/silence_grape_warnings Silence Grape warnings * Merge pull request #2017 from bf4/remove_warnings Fix mt6 assert_nil warnings * Updated isolated tests to assert correct behavior. (#2010) * Updated isolated tests to assert correct behavior. * Added check to get unsafe params if rails version is great than 5 * Merge pull request #2012 from bf4/cleanup_isolated_jsonapi_renderer_tests_a_bit Cleanup assertions in isolated jsonapi renderer tests a bit * Add Model#attributes helper; make test attributes explicit * Fix model attributes accessors * Fix typos * Randomize testing of compatibility layer against regressions * Test bugfix * Add CHANGELOG * Merge pull request #1981 from groyoh/link_doc Fix relationship links doc Conflicts: CHANGELOG.md

* Merge pull request rails-api#1990 from mxie/mx-result-typo Fix typos and capitalization in Relationship Links docs [ci skip] * Merge pull request rails-api#1992 from ojiry/bump_ruby_versions Run tests by Ruby 2.2.6 and 2.3.3 * Merge pull request rails-api#1994 from bf4/promote_architecture Promote important architecture description that answers a lot of questions we get Conflicts: docs/ARCHITECTURE.md * Merge pull request rails-api#1999 from bf4/typos Fix typos [ci skip] * Merge pull request rails-api#2000 from berfarah/patch-1 Link to 0.10.3 tag instead of `master` branch * Merge pull request rails-api#2007 from bf4/check_ci Test was failing due to change in JSON exception message when parsing empty string * Swap out KeyTransform for CaseTransform (rails-api#1993) * delete KeyTransform, use CaseTransform * added changelog Conflicts: CHANGELOG.md * Merge pull request rails-api#2005 from kofronpi/support-ruby-2.4 Update jsonapi runtime dependency to 0.1.1.beta6 * Bump to v0.10.4 * Merge pull request rails-api#2018 from rails-api/bump_version Bump to v0.10.4 [ci skip] Conflicts: CHANGELOG.md * Merge pull request rails-api#2019 from bf4/fix_method_redefined_warning Fix AMS warnings * Merge pull request rails-api#2020 from bf4/silence_grape_warnings Silence Grape warnings * Merge pull request rails-api#2017 from bf4/remove_warnings Fix mt6 assert_nil warnings * Updated isolated tests to assert correct behavior. (rails-api#2010) * Updated isolated tests to assert correct behavior. * Added check to get unsafe params if rails version is great than 5 * Merge pull request rails-api#2012 from bf4/cleanup_isolated_jsonapi_renderer_tests_a_bit Cleanup assertions in isolated jsonapi renderer tests a bit * Add Model#attributes helper; make test attributes explicit * Fix model attributes accessors * Fix typos * Randomize testing of compatibility layer against regressions * Test bugfix * Add CHANGELOG * Merge pull request rails-api#1981 from groyoh/link_doc Fix relationship links doc Conflicts: CHANGELOG.md

groyoh added the C: Documentation label Nov 20, 2016

bf4 reviewed Nov 21, 2016

View reviewed changes

bf4 mentioned this pull request Nov 21, 2016

url_helpers non-functional in links method? #1971

Open

groyoh force-pushed the link_doc branch from 09504f9 to 3326d12 Compare November 22, 2016 17:12

bf4 reviewed Nov 27, 2016

View reviewed changes

docs/howto/add_link.md Show resolved Hide resolved

bf4 reviewed Nov 27, 2016

View reviewed changes

bf4 added C: JSON API S: Changelog Missing V: 0.10.x labels Nov 27, 2016

groyoh commented Nov 27, 2016

View reviewed changes

groyoh force-pushed the link_doc branch from 3326d12 to ad5b7bd Compare December 30, 2016 19:56

groyoh changed the title ~~Update pagination and links doc~~ Fix relationship links doc Dec 30, 2016

groyoh force-pushed the link_doc branch from ad5b7bd to e3c7b5a Compare December 30, 2016 19:59

[skip ci] Fix relationship link documentation

91128fa

groyoh force-pushed the link_doc branch from e3c7b5a to 91128fa Compare December 30, 2016 20:02

groyoh removed the S: Changelog Missing label Dec 30, 2016

bf4 reviewed Jan 7, 2017

View reviewed changes

bf4 requested changes Jan 7, 2017

View reviewed changes

bf4 approved these changes Jan 10, 2017

View reviewed changes

bf4 merged commit 2a6d373 into rails-api:master Jan 10, 2017

This was referenced Jan 10, 2017

make serialization_context available in link #1976

Closed

Update docs for JSONAPI relationship links #1935

Open

bf4 added a commit that referenced this pull request Jan 10, 2017

Merge pull request #1981 from groyoh/link_doc

9a2f489

Fix relationship links doc Conflicts: CHANGELOG.md

		@@ -22,10 +22,10 @@ This is the documentation of ActiveModelSerializers, it's focused on the **0.10.

		## How to

		- [How to use ActiveModelSerializers outside of rails controllers](howto/outside_controller_use.md)


		`ActiveModelSerializers` offers you many ways to add links in your JSON reponse depending on the adapter you are using.

		Note that within the following examples, we set the `adapter` option, but that is not needed if the `ActiveModelSerializers.config.adapter` variable is properly set. Also, some examples are given for both Rails and [non-Rails usage](outside_controller_use.md).


		## JSON adapter

		The `json` is not able to handle links defined by the `link` class method. However, you can either use the `meta` member or define a `links` attribute on the serializer.


		## Attributes adapter

		The only way to provide pagination with the `attributes` adapter is to define a `links` attribute [as described for the JSON adapter above](#via-an-attribute).


		## JSON API adapter

		If you want to paginate a collection with the `json_api` adapter, you can either use the top-level `links` member to provide pagination links or the top-level `meta` member to provide pagination information:

Fix relationship links doc #1981

Fix relationship links doc #1981

Conversation

groyoh commented Nov 20, 2016 • edited

Purpose

Changes

Related GitHub issues

Additional helpful information

mention-bot commented Nov 20, 2016

michael-reeves commented Nov 21, 2016

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

bf4 commented Nov 21, 2016

bf4 commented Nov 21, 2016

groyoh commented Nov 21, 2016

michael-reeves commented Nov 21, 2016

groyoh commented Nov 22, 2016

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

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

bf4 Nov 27, 2016 • edited

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

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

bf4 commented Nov 27, 2016

Choose a reason for hiding this comment

groyoh commented Dec 30, 2016

Choose a reason for hiding this comment

groyoh Jan 7, 2017 • edited

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

bf4 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bf4 commented Jan 10, 2017

groyoh commented Nov 20, 2016 •

edited

bf4 Nov 27, 2016 •

edited

groyoh Jan 7, 2017 •

edited