GO-370 Signing tags #227

mirrec · 2023-12-04T15:41:47Z

Nedaval som tu zatial ziadnu abstrakciu, len natvrdo sa zavola metoda v callbacku.
Pridal som unique indexy, lebo cisto teoreticky by nam tam mohli nejako povznikat aj duplikaty a to by nas neskor mohlo strielat do nohy.

jsuchal

Trosku treba este postruhat

jsuchal · 2023-12-04T20:28:00Z

app/models/tenant.rb


  has_many :boxes, dependent: :destroy
  has_many :automation_rules, class_name: "Automation::Rule", dependent: :destroy
  has_many :tags, dependent: :destroy
+  has_many :signature_requested_to_tags


asi by som pouzil signature_requested_from_tags aleb to_be_signed_by_tags lebo toto znie divne.

a tu sa bavime iba o nazve tej asociacie z pohladu tenanta? alebo riesis (aj) nazov classy?

signature_requested_from_tags nie, lebo to hovori, ze od koho ta poziadavka prisla a my chceme presne opacnu vec: na koho / ku komu prisla. a tam je to request to
https://www.quora.com/Which-is-correct-request-to-or-request-for-in-English

to_be_signed_by_tags mi dava zmysel. ale potom aj ten vseobecny podpisovaci stitok "na podpis" pomenujem ToBeSignedTag

Podla mna to motas.

Signature Request From - ziadam podpis od niekoho
Signature Requested From - podpis bol od niekoho ziadany (minuly cas)
Signature Requested To - podla mna neznamena nic - ani tvoj link takyto tvar nepozna.
Request To Sign - o tomto sa pise v clanku co linkujes.
(Data) To Be Signed - toto je z autogramu z europskej kniznice DSS.

jsuchal · 2023-12-04T20:30:10Z

db/migrate/20231204133308_add_user_id_to_tags.rb

@@ -0,0 +1,26 @@
+class AddUserIdToTags < ActiveRecord::Migration[7.0]
+  def up
+    add_column :tags, :user_id, :integer


Len taky napad, preco tu vlastne nevyuzijeme to, ze mame user groupy a naviazeme ich cez existujucu logiku?

jsuchal · 2023-12-04T20:34:05Z

app/models/signer_group.rb

+    tag.save!
+  end
+
+  def self.user_removed_from_group(signer_group, user)


Toto je myslim, nestastne nazvane lebo v skutocnosti tu chces povedat, ze destroy_all_signature_requests_for(group_membership)

jsuchal · 2023-12-04T20:35:45Z

app/models/signer_group.rb

@@ -14,4 +14,20 @@ class SignerGroup < Group
  def name
    I18n.t("group.names.signer")
  end
+
+  def self.user_added_to_group(signer_group, user)


Tu to je nestastne nazvane lebo chces skor povedat, ze create_signing_tags_for(group_membership) a ten si to vyberie ako treba. Ja by som zvazil toto dat normalne do tej GroupMembership triedy. Je to Core a tu mi to uplne nesedi kedze to je naviazane vsade na group membership.

jsuchal · 2023-12-04T20:36:22Z

test/fixtures/tags.yml

+ssd_signature_requested:
+  name: Na podpis
+  type: SignatureRequestedTag
+  visible: true
+  tenant: ssd
+
+ssd_signed:
+  name: Podpisane
+  type: SignedTag
+  visible: true
+  tenant: ssd
+


Ak chceme testovat ze to vyrobi, tak to nedavajme do fixtures lebo sa popalime ako minule pri delivery tagu.

toto su ale vseobecne podpisove stitky. nie su viazane na konkretneho usera. toto je to iste ako DraftTag, cize za mna by to tu malo byt. lebo sa to vytvara, ked sa tvori tenant, a ten uz mame.

Tak potom ale pridajme test co naozaj vytvori tenanta

jsuchal · 2023-12-06T18:28:46Z

app/models/group_membership.rb

+
+  after_create do |group_membership|
+    if group_membership.group.is_a?(SignerGroup)
+      self.class.create_signing_tags_for(group_membership)


co keby sme toto hodili rovno do group_membership.create_signing_tags! ?

jsuchal · 2023-12-06T18:29:12Z

app/models/group_membership.rb

+
+  after_destroy do |group_membership|
+    if group_membership.group.is_a?(SignerGroup)
+      self.class.destroy_all_signature_requests_for(group_membership)


aj toto mi sedi asi viac do group_membership priamo.

jsuchal · 2023-12-06T18:30:14Z

app/models/group_membership.rb

+    find_or_create_signing_tag(
+      tags_scope: tenant.signature_requested_from_tags,
+      user_group: user_group,
+      tag_name: "Na podpis - #{user.name}"


Vo figme myslim neboli pomlcky

app/models/signature_requested_from_tag.rb

jsuchal · 2023-12-06T18:38:50Z

app/models/group_membership.rb

+    tag.destroy if tag
+  end
+
+  def self.find_or_create_signing_tag(tags_scope:, user_group:, tag_name:)


Toto je podla mna len strukturalna podobnost a nie skutocna abstrakcia biznisova. tag_scope nie je nic biznisove a matie ma to a zaroven tam mas skaredy || co tomu len pridava magiu.

ja potrebujem poriesit dve veci.

aby som nasiel tag, ktory ma userovu skupinu (lebo napriklad, ked niekto bol podpisovac, ale potom ho dali prec, tak chceme aby tam tie jeho podpisane tagy ostali zachovane. zaroven, ked ho znova niekto prida do skupiny, tak nechcem vytvarat duplicitnu skupinu, preto hladam podla user groupy)

nazvy skupin musia byt unikatne, meno usera sa mohlo menit.

toto riesi oba pripady.

v testoch je to inak vidno, ktore pripady to riesi.

jsuchal · 2023-12-06T18:41:00Z

app/models/group_membership.rb

+    end
+  end
+
+  def self.create_signing_tags_for(group_membership)


Keby si tu nazval group_membership ako signer_group_membership tak by sa kopec veci vyjasnilo. Inac je to tu sama skupina a user, nevyzna sa v tom ani diva svina.

kedze to us je instancna metoda, tento problem odpada.

jsuchal · 2023-12-06T18:42:56Z

app/models/group_membership.rb

+    tenant = group_membership.group.tenant
+
+    find_or_create_signing_tag(
+      tags_scope: tenant.signature_requested_from_tags,


tejto magii uplne nerozumiem, co je zle na napr
tenant.signature_requested_from_tags.find_or_create_by!(...) ?

vysvetlene vyssie v komente, potrebujem najst podla groupy, lebo stitok so skupinou moze existovat (pridanie / odobratie / pridanie toho isteho usera), a potom unique name podla nazvu (to teoreticky mozeme vypustit, len mne to vyskakovalo pri implementacii, ked som sa s tym hral a nieco sa nepodarilo uplne vytvorit).

jsuchal · 2023-12-06T18:43:46Z

app/models/signer_group.rb

@@ -14,4 +14,8 @@ class SignerGroup < Group
  def name
    I18n.t("group.names.signer")
  end
+
+  def self.user_removed_from_group(signer_group, user)


Toto ti tu ostalo navyse? Nevidim to nikde naviazane na nic.

jsuchal · 2023-12-06T18:46:36Z

app/models/tag.rb

@@ -48,4 +48,8 @@ def gives_access?
  def destroyable?
    raise NotImplementedError
  end
+
+  def self.find_tag_containing_group(group)
+    includes(:groups).to_a.find { |tag| tag.groups.include?(group) }


Toto je nejaka prasarna nie? Sice to budu self joiny a aliasy ale toto sa da normalne cez sql spravit. Minimalne cez nejaky where scope nad tag to musi ist. Staci ti tam vlozit subquery

ano, da sa to spravit vsetko cez sql. akurat mi to pride nateraz zbytocne. teraz to ma byt funkcne, a potom ked bude podpisovanie, tak pome optimalizovat query. navyse sa to bude volat velmi malo krat.

jsuchal · 2023-12-07T07:36:37Z

app/models/group_membership.rb

+  after_create :create_signing_tags!, if: Proc.new { |membership| membership.group.is_a?(SignerGroup) }
+  after_destroy :destroy_all_signature_requests!, if: Proc.new { |membership| membership.group.is_a?(SignerGroup) }


Myslim, ze drzime konvenciu

Suggested change

after_create :create_signing_tags!, if: Proc.new { |membership| membership.group.is_a?(SignerGroup) }

after_destroy :destroy_all_signature_requests!, if: Proc.new { |membership| membership.group.is_a?(SignerGroup) }

after_create :create_signing_tags!, if: ->(membership) { membership.group.is_a?(SignerGroup) }

after_destroy :destroy_all_signature_requests!, if: ->(membership) { membership.group.is_a?(SignerGroup) }

https://guides.rubyonrails.org/active_record_callbacks.html#using-if-and-unless-with-a-symbol

jsuchal · 2023-12-07T07:38:45Z

db/migrate/20231204133308_generate_signers_tags.rb

+class GenerateSignersTags < ActiveRecord::Migration[7.0]
+  def up
+    SignerGroup.includes(group_memberships: [:group, :user]).find_each do |group|
+      group.group_memberships.map(&:create_signing_tags!)


Suggested change

group.group_memberships.map(&:create_signing_tags!)

group.group_memberships.each(&:create_signing_tags!)

jsuchal · 2023-12-07T07:39:28Z

test/models/tenant_test.rb

+require "test_helper"
+
+class TenantTest < ActiveSupport::TestCase
+  test "all system tags and groups are created with tenant" do


mirrec added 3 commits December 4, 2023 15:13

Create group membership though tenant

bd761a5

Handle creation and removing signing tags

b1274af

Add simple unit test

8b033c3

mirrec requested a review from jsuchal December 4, 2023 15:41

jsuchal requested changes Dec 4, 2023

View reviewed changes

mirrec added 5 commits December 6, 2023 14:48

Merge branch 'main' into GO-370/signing-tags

7031cf7

Identify signing tags by user group

e0a579c

Use headless_chrome

d991482

Move controller file to to right location

91ed09b

Merge branch 'main' into GO-370/signing-tags

73b5500

mirrec requested a review from jsuchal December 6, 2023 15:26

mirrec added 4 commits December 6, 2023 16:30

Merge branch 'main' into GO-370/signing-tags

931f0ba

Update schema after migration

9317fbd

Use after_create

a90e60f

Improve test coverage

0bd4bd0

jsuchal requested changes Dec 6, 2023

View reviewed changes

Apply feedback

8fcef35

mirrec requested a review from jsuchal December 6, 2023 22:50

jsuchal reviewed Dec 7, 2023

View reviewed changes

Apply feedback

eb03322

mirrec requested a review from jsuchal December 7, 2023 07:46

jsuchal approved these changes Dec 7, 2023

View reviewed changes

mirrec merged commit 6040637 into main Dec 7, 2023
3 checks passed

mirrec deleted the GO-370/signing-tags branch December 7, 2023 07:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GO-370 Signing tags #227

GO-370 Signing tags #227

mirrec commented Dec 4, 2023

jsuchal left a comment

jsuchal Dec 4, 2023

mirrec Dec 6, 2023

jsuchal Dec 6, 2023

mirrec Dec 6, 2023

jsuchal Dec 6, 2023

jsuchal Dec 4, 2023

jsuchal Dec 4, 2023

jsuchal Dec 4, 2023

jsuchal Dec 4, 2023

mirrec Dec 6, 2023

jsuchal Dec 6, 2023

jsuchal Dec 6, 2023

jsuchal Dec 6, 2023

jsuchal Dec 6, 2023

jsuchal Dec 6, 2023

mirrec Dec 6, 2023

mirrec Dec 6, 2023

jsuchal Dec 6, 2023

mirrec Dec 6, 2023

jsuchal Dec 6, 2023

mirrec Dec 6, 2023

jsuchal Dec 6, 2023

jsuchal Dec 6, 2023

mirrec Dec 6, 2023 •

edited

Loading

jsuchal Dec 7, 2023

mirrec Dec 7, 2023

jsuchal Dec 7, 2023

jsuchal Dec 7, 2023

		after_create :create_signing_tags!, if: Proc.new { \|membership\| membership.group.is_a?(SignerGroup) }
		after_destroy :destroy_all_signature_requests!, if: Proc.new { \|membership\| membership.group.is_a?(SignerGroup) }

	group.group_memberships.map(&:create_signing_tags!)
	group.group_memberships.each(&:create_signing_tags!)

GO-370 Signing tags #227

GO-370 Signing tags #227

Conversation

mirrec commented Dec 4, 2023

jsuchal left a 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

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

mirrec Dec 6, 2023 • 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

mirrec Dec 6, 2023 •

edited

Loading