Add support for alternative IDs using Hashids #2564

gabrielgiroe1 · 2024-03-06T14:28:06Z

Description

Fixes #2563

Checklist:

I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have added tests that prove my fix is effective or that my feature works

Screenshots & recording

Manual review steps

Step 1
Step 2

Manual reviewer: please leave a comment with output from the test if that's the case.

Gemfile

codeclimate · 2024-03-06T14:29:26Z

Code Climate has analyzed commit d58b9c6 and detected 0 issues on this pull request.

View more on Code Climate.

spec/dummy/app/avo/resources/city.rb

…n.rb

github-actions · 2024-04-03T01:56:47Z

This PR has been marked as stale because there was no activity for the past 15 days.

adrianthedev · 2024-04-15T07:31:23Z

What is the status on this one?

adrianthedev · 2024-04-18T17:26:49Z

@gabrielgiroe1 what is the status on this one?

git reset

Paul-Bob · 2024-04-19T07:15:24Z

The real ID is leaking on attach request params, let's try to find out more spots like this one and fix them.

…2749) Bumps the npm_and_yarn group with 1 update in the / directory: [trix](https://github.com/basecamp/trix). Updates `trix` from 2.1.0 to 2.1.1 - [Release notes](https://github.com/basecamp/trix/releases) - [Commits](basecamp/trix@v2.1.0...v2.1.1) --- updated-dependencies: - dependency-name: trix dependency-type: direct:production dependency-group: npm_and_yarn ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Co-authored-by: depfu[bot] <23717796+depfu[bot]@users.noreply.github.com> Co-authored-by: Paul Bob <69730720+Paul-Bob@users.noreply.github.com> Co-authored-by: Adrian Marin <adrian@adrianthedev.com>

git reset

Gemfile

* tweaks * . * lint * . * test * tweak * revert tests on special branch

adrianthedev

The PR is generally ok.
I have a couple of questions and one more critical change request.

Good job taking it this far!

lib/avo/plugin_manager.rb

lib/avo/fields/belongs_to_field.rb

app/components/avo/fields/belongs_to_field/edit_component.html.erb

lib/avo/base_resource.rb

adrianthedev · 2024-05-28T10:52:05Z

lib/avo/fields/belongs_to_field.rb

+      def target_resource(record: @record, polymorphic_model_class: value&.class)
        @target_resource ||= if use_resource.present?


This is not right.
We can't memoize a value in a method if the method takes arguments.

If one time the record is a User, it will get memoized as that user. Then, the second time when the record is a team, it will return the first user, resulting in a bug.

Did I read that correctly?

adrianthedev · 2024-05-28T10:53:17Z

lib/avo/fields/belongs_to_field.rb

@@ -229,22 +238,22 @@ def database_id
        id
      end

-      def target_resource
+      def target_resource(record: @record, polymorphic_model_class: value&.class)


Also, I'd like to maintain target_resource as a method without params so we can quickly call field.target_resource when we need to.

Add support for alternative IDs using Hashids

bde932d

github-actions bot added the Feature label Mar 6, 2024

gabrielgiroe1 marked this pull request as draft March 6, 2024 14:28

github-actions bot reviewed Mar 6, 2024

View reviewed changes

Gemfile Outdated Show resolved Hide resolved

Gemfile Outdated Show resolved Hide resolved

Gemfile Outdated Show resolved Hide resolved

gabrielgiroe1 added 4 commits March 6, 2024 16:32

Fix rubocop

a92973f

Remove Hashid inclusion from ApplicationRecord

89d4e2f

Add Hashid inclusion

d4a25be

Fix error for hashid

065920f

github-actions bot reviewed Mar 6, 2024

View reviewed changes

spec/dummy/app/avo/resources/city.rb Outdated Show resolved Hide resolved

gabrielgiroe1 marked this pull request as ready for review March 7, 2024 10:19

gabrielgiroe1 added Feature and removed Feature labels Mar 7, 2024

gabrielgiroe1 added 3 commits March 7, 2024 15:09

Fix lint error

f549b7f

Add hashid-rails on development test

1303a92

Add require statement for hashid-rails gem in application.rb

976f49d

Paul-Bob assigned gabrielgiroe1 Mar 11, 2024

Remove unnecessary require statement for 'hashid/rails' in applicatio…

3760152

…n.rb

github-actions bot added the Stale label Apr 3, 2024

Paul-Bob removed the Stale label Apr 3, 2024

Paul-Bob marked this pull request as draft April 18, 2024 09:34

Update gemfile

6587cf8

gabrielgiroe1 marked this pull request as ready for review April 18, 2024 20:36

gabrielgiroe1 and others added 3 commits April 18, 2024 23:53

Resolve conflict

cd2943d

git reset

Merge branch 'main' into feature/support-alternative-ids

4cdb5ae

Add gemfiles

2bfacff

Paul-Bob and others added 10 commits May 8, 2024 20:51

Bumped avo to 3.7.0

7001a03

chore: require ostruct (#2750)

dfbc264

[js] Update all Yarn dependencies (2024-04-25) (#2720)

b50e340

Co-authored-by: depfu[bot] <23717796+depfu[bot]@users.noreply.github.com> Co-authored-by: Paul Bob <69730720+Paul-Bob@users.noreply.github.com> Co-authored-by: Adrian Marin <adrian@adrianthedev.com>

wip

5bf6edb

feature: heading target (#2753)

9d30c43

Update Gemfile

4b49ad4

Resolve conflict

dda5072

git reset

Update Gemfile

b849c84

Merge branch 'main' into feature/support-alternative-ids

a9fbed4

github-actions bot reviewed May 8, 2024

View reviewed changes

Gemfile Outdated Show resolved Hide resolved

gabrielgiroe1 and others added 6 commits May 8, 2024 20:58

Removed duplication

a28eb05

Removed duplication

d7a565f

wip

017f5a4

Merge branch 'main' into feature/support-alternative-ids

7e71c31

wip

8be554f

wip

3094efc

Paul-Bob mentioned this pull request May 16, 2024

tweaks for https://github.com/avo-hq/avo/pull/2564 #2776

Merged

tweaks for #2564 (#2776)

65fbfba

* tweaks * . * lint * . * test * tweak * revert tests on special branch

gabrielgiroe1 requested a review from Paul-Bob May 16, 2024 14:56

Paul-Bob approved these changes May 16, 2024

View reviewed changes

gabrielgiroe1 requested a review from adrianthedev May 17, 2024 08:13

Merge branch 'main' into feature/support-alternative-ids

c6fc4d0

adrianthedev requested changes May 28, 2024

View reviewed changes

gabrielgiroe1 and others added 3 commits May 28, 2024 14:56

wip

7af4de9

Merge branch 'main' into feature/support-alternative-ids

5c9af6b

wip

add775e

adrianthedev mentioned this pull request May 29, 2024

Added prefixed_ids and hashid avo-hq/main.avodemo.com#20

Open

gabrielgiroe1 and others added 2 commits May 30, 2024 14:52

wip

f099acf

Merge branch 'main' into feature/support-alternative-ids

d58b9c6

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for alternative IDs using Hashids #2564

Add support for alternative IDs using Hashids #2564

gabrielgiroe1 commented Mar 6, 2024 •

edited

codeclimate bot commented Mar 6, 2024 •

edited

github-actions bot commented Apr 3, 2024

adrianthedev commented Apr 15, 2024

adrianthedev commented Apr 18, 2024

Paul-Bob commented Apr 19, 2024

adrianthedev left a comment

adrianthedev May 28, 2024

adrianthedev May 28, 2024

		def target_resource(record: @record, polymorphic_model_class: value&.class)
		@target_resource \|\|= if use_resource.present?

Add support for alternative IDs using Hashids #2564

Are you sure you want to change the base?

Add support for alternative IDs using Hashids #2564

Conversation

gabrielgiroe1 commented Mar 6, 2024 • edited

Description

Checklist:

Screenshots & recording

Manual review steps

codeclimate bot commented Mar 6, 2024 • edited

github-actions bot commented Apr 3, 2024

adrianthedev commented Apr 15, 2024

adrianthedev commented Apr 18, 2024

Paul-Bob commented Apr 19, 2024

adrianthedev left a comment

Choose a reason for hiding this comment

adrianthedev May 28, 2024

Choose a reason for hiding this comment

adrianthedev May 28, 2024

Choose a reason for hiding this comment

gabrielgiroe1 commented Mar 6, 2024 •

edited

codeclimate bot commented Mar 6, 2024 •

edited