Summary

Use automergeAttrs for BaseResource and subclasses.

Review Request #13853 — Created May 13, 2024 and submitted May 21, 2024, 4 p.m.

Information

Owner

david

Repository

Review Board

Branch

release-7.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

This change makes use of automergeAttrs for our resource classes.
Spina itself specifies this for defaults, which means we don't have to
manually call _.defaults() for all of these. This additionally makes
use of it for attrToJsonMap, deserializers, and serializers.

Testing Done


Ran js-tests.
Verified that all objects were properly automerged.

Commits

Summary	ID
Use automergeAttrs for BaseResource and subclasses. This change makes use of `automergeAttrs` for our resource classes. Spina itself specifies this for `defaults`, which means we don't have to manually call `_.defaults()` for all of these. This additionally makes use of it for `attrToJsonMap`, `deserializers`, and `serializers`. Testing Done: - Ran js-tests. - Verified that all objects were properly automerged.	d732b84d5bee784659db9673c7b4f8ef9e8be876

Summary

Use automergeAttrs for BaseResource and subclasses.

This change makes use of `automergeAttrs` for our resource classes. Spina itself specifies this for `defaults`, which means we don't have to manually call `_.defaults()` for all of these. This additionally makes use of it for `attrToJsonMap`, `deserializers`, and `serializers`. Testing Done: - Ran js-tests. - Verified that all objects were properly automerged.

d732b84d5bee784659db9673c7b4f8ef9e8be876

Issues

Description	From	Last Updated
Does this not work as: static defaults(): Result<Partial<BaseCommentAttrs>> { ... } It should accept both and merge that correctly. Also, …	chipx86	May 21, 2024, 8:37 a.m.
This can just be a static result now, I think.	chipx86	May 21, 2024, 4 p.m.

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

reviewboard/static/rb/js/common/resources/models/baseCommentModel.ts (Diff revision 1)

The issue has been resolved. Show all issues

Does this not work as:

static defaults(): Result<Partial<BaseCommentAttrs>> {
    ...
}


It should accept both and merge that correctly.
Also, Spina exposes its own Result, so no need for the Backbone._Result.

Change Summary:


Use spina's Result
Fix method definitions to not use arrow functions.
Add types to subclass' definitions so we don't get warnings about incompatibility.
Make listKey a prototypeAttr, which it should have always been.

Commits:

	Summary	ID
	Use automergeAttrs for BaseResource and subclasses. This change makes use of `automergeAttrs` for our resource classes. Spina itself specifies this for `defaults`, which means we don't have to manually call `_.defaults()` for all of these. This additionally makes use of it for `attrToJsonMap`, `deserializers`, and `serializers`. Testing Done: - Ran js-tests. - Verified that all objects were properly automerged.	059f82cdd65a0b9b53b67180eba170148a991bd7
	Use automergeAttrs for BaseResource and subclasses. This change makes use of `automergeAttrs` for our resource classes. Spina itself specifies this for `defaults`, which means we don't have to manually call `_.defaults()` for all of these. This additionally makes use of it for `attrToJsonMap`, `deserializers`, and `serializers`. Testing Done: - Ran js-tests. - Verified that all objects were properly automerged.	d732b84d5bee784659db9673c7b4f8ef9e8be876

Diff:

Revision 2 (+198 -132)

Show changes

	reviewboard/static/rb/js/common/resources/models/baseCommentModel.ts
	reviewboard/static/rb/js/common/resources/models/baseResourceModel.ts
	reviewboard/static/rb/js/common/resources/models/draftReviewModel.ts
	reviewboard/static/rb/js/common/resources/models/fileAttachmentModel.ts
	reviewboard/static/rb/js/common/resources/models/reviewModel.ts
	reviewboard/static/rb/js/common/resources/models/reviewReplyModel.ts
	reviewboard/static/rb/js/common/resources/models/reviewRequestModel.ts

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

reviewboard/static/rb/js/common/resources/models/draftReviewModel.ts (Diff revision 2)

The issue has been resolved. Show all issues

This can just be a static result now, I think.

Status:: Completed
Change Summary:: Pushed to release-7.x (117297c)