Skip to content

Next

  • Projects
  • Groups
  • Snippets
  • Help
    • Loading...
    • Help
    • Support
    • Submit feedback
    • Contribute to GitLab
    • Switch to GitLab Next
  • Sign in / Register
Minds Backend - Engine
Minds Backend - Engine
  • Project
    • Project
    • Details
    • Activity
    • Releases
    • Cycle Analytics
    • Insights
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
    • Charts
    • Locked Files
  • Issues 222
    • Issues 222
    • List
    • Boards
    • Labels
    • Service Desk
    • Milestones
  • Merge Requests 33
    • Merge Requests 33
  • CI / CD
    • CI / CD
    • Pipelines
    • Jobs
    • Schedules
    • Charts
  • Security & Compliance
    • Security & Compliance
    • Dependency List
  • Packages
    • Packages
    • List
    • Container Registry
  • Wiki
    • Wiki
  • Snippets
    • Snippets
  • Members
    • Members
  • Collapse sidebar
  • Activity
  • Graph
  • Charts
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
  • Minds
  • Minds Backend - EngineMinds Backend - Engine
  • Merge Requests
  • !303

Open
Opened 2 weeks ago by Guy Thouret@gthouret
  • Report abuse
Report abuse

Propagate Properties when entities get updated

Edited 1 week ago by Guy Thouret

Check out, review, and merge locally

Step 1. Fetch and check out the branch for this merge request

git fetch origin
git checkout -b "chore/616-propogate-props" "origin/chore/616-propogate-props"

Step 2. Review the changes locally

Step 3. Merge the branch and fix any conflicts that come up

git fetch origin
git checkout "origin/master"
git merge --no-ff "chore/616-propogate-props"

Step 4. Push the result of the merge to GitLab

git push origin "master"

Note that pushing to GitLab requires write access to this repository.

Tip: You can also checkout merge requests locally by following these guidelines.

Request to merge chore/616-propogate-props into master
The source branch is 33 commits behind the target branch
Open in Web IDE
  • Email patches
  • Plain diff
Pipeline #81925893 passed for 31cffea2 on chore/616-propogate-props
          Requires 2 more approvals from Devs and Deployers.
          Brian Hatchet
          Brian Hatchet
          Ben Hayward
          Ben Hayward
          Emiliano Balbuena
          Emiliano Balbuena
          Martin Santangelo
          Martin Santangelo
          Rami Albatal
          Rami Albatal
          There are merge conflicts. Resolve these conflicts or ask someone with write access to this repository to merge it locally
          You can merge this merge request manually using the
          • Discussion 14
          • Commits 22
          • Pipelines 15
          • Changes 29
          3/7 threads resolved
          • Loading...
          • Guy Thouret @gthouret added MR::Awaiting Review Priority::2 - Normal Squad::Blue scoped labels 2 weeks ago

            added MR::Awaiting Review Priority::2 - Normal Squad::Blue scoped labels

          • Guy Thouret @gthouret added 3 commits 2 weeks ago

            added 3 commits

            • 66727176 - (feat) Tidy interface of PropogateProperties class to a single public function - #616 (closed)
            • 7cea5cdd - (feat) Refactor code that is propagating properties into the propagation framework - #616 (closed)
            • 555d3a4f - (chore) Remove duplicate doc blocks for methods that already have their own doc blocks - #616 (closed)

            Compare with previous version

          • Guy Thouret @gthouret added 12 commits 2 weeks ago

            added 12 commits

            • 555d3a4f...a437ec98 - 11 commits from branch master
            • d720d192 - Merge branch 'master' into chore/616-propogate-props

            Compare with previous version

          • Guy Thouret @gthouret added 3 commits 2 weeks ago

            added 3 commits

            • 4ad8b92c - (chore) Linting new PropogateProperties classes - #616 (closed)
            • 980d6ed4 - (fix) Typo changing all instances propogate -> propagate - #616 (closed)
            • 12402bb2 - (fix) Regeressions in existing spec tests - #616 (closed)

            Compare with previous version

          • Guy Thouret @gthouret added 1 commit 2 weeks ago

            added 1 commit

            • 4d3e9456 - (chore) Add missing spec tests for new classes; Refactor usage to not use...

            Compare with previous version

          • Guy Thouret @gthouret added 1 commit 2 weeks ago

            added 1 commit

            • 0032c435 - (chore) Lint recent changes - #616 (closed)

            Compare with previous version

          • Mark Harding
            Mark Harding @markeharding started a thread on an old version of the diff 2 weeks ago
            Last updated by Guy Thouret 2 weeks ago
            Core/Entities/PropagateProperties.php
            29 $this->save = $save ?? new Save();
            30 $this->entitiesBuilder = $entitiesBuilder ?? Di::_()->get('EntitiesBuilder');
            31 $this->registerPropagators();
            32 }
            33
            34 protected function registerPropagators(): void
            35 {
            36 /* Register PropertyPropagator classes here */
            37 $this->addPropagator(Core\Blogs\Delegates\PropagateProperties::class);
            38 $this->addPropagator(Core\Feeds\Delegates\PropagateProperties::class);
            39 $this->addPropagator(Core\Media\Delegates\PropagateProperties::class);
            40 $this->addPropagator(Core\Entities\Delegates\PropagateProperties::class);
            41 $this->addPropagator(Core\Permissions\Delegates\PropagateProperties::class);
            42 }
            43
            44 public function clearPropogators()
            • Mark Harding
              Mark Harding @markeharding · 2 weeks ago
              Owner

              void return type?

            • Guy Thouret
              Guy Thouret @gthouret · 2 weeks ago
              Developer

              It's means no return type is expected, throws an error if you try and return anything from it. Not strictly necessary but I'm just in the habi of adding return types to any code I write where possible.

            • Mark Harding
              Mark Harding @markeharding · 2 weeks ago
              Owner

              I was suggesting to put one in here as it doesn't exist

            • Guy Thouret @gthouret changed this line in version 8 of the diff 2 weeks ago

              changed this line in version 8 of the diff

            • Please register or sign in to reply
          • Mark Harding
            Mark Harding @markeharding started a thread on the diff 2 weeks ago
            Resolved by Mark Harding 4 days ago
          • Mark Harding
            Mark Harding @markeharding started a thread on the diff 2 weeks ago
            Core/Blogs/Delegates/PropagateProperties.php
            19 if ($this->valueHasChanged($blurb, $to->get('blurb'))) {
            20 $to->set('blurb', $blurb);
            21 }
            22
            23 if ($this->valueHasChanged($from->getUrl(), $to->getURL())) {
            24 $to->setURL($from->getUrl());
            25 }
            26
            27 if ($this->valueHasChanged($from->getIconUrl(), $to->get('thumbnail_src'))) {
            28 $to->set('thumbnail_src', $from->getIconUrl());
            29 }
            30
            31 return $to;
            32 }
            33
            34 public function fromActivity(Activity $from, $to)
            • Mark Harding
              Mark Harding @markeharding · 2 weeks ago
              Owner

              Return type?

            • Please register or sign in to reply
          • Mark Harding
            Mark Harding @markeharding started a thread on the diff 2 weeks ago
            Resolved by Mark Harding 4 days ago
          • Mark Harding
            Mark Harding @markeharding started a thread on the diff 2 weeks ago
            Core/Blogs/Delegates/PropagateProperties.php
            19 if ($this->valueHasChanged($blurb, $to->get('blurb'))) {
            20 $to->set('blurb', $blurb);
            21 }
            22
            23 if ($this->valueHasChanged($from->getUrl(), $to->getURL())) {
            24 $to->setURL($from->getUrl());
            25 }
            26
            27 if ($this->valueHasChanged($from->getIconUrl(), $to->get('thumbnail_src'))) {
            28 $to->set('thumbnail_src', $from->getIconUrl());
            29 }
            30
            31 return $to;
            32 }
            33
            34 public function fromActivity(Activity $from, $to)
            • Mark Harding
              Mark Harding @markeharding · 2 weeks ago
              Owner

              Throw exception if called?

            • Please register or sign in to reply
          • Mark Harding
            Mark Harding @markeharding started a thread on the diff 2 weeks ago
            Resolved by Mark Harding 4 days ago
          • Mark Harding
            Mark Harding @markeharding started a thread on an old version of the diff 2 weeks ago
            Last updated by Guy Thouret 2 weeks ago
            Core/Entities/Propagator/Properties.php
            1 <?php
            2
            3 namespace Minds\Core\Entities\Propagator;
            4
            5 use Minds\Entities\Activity;
            6
            7 abstract class Properties
            8 {
            9 protected $actsOnType = 'any';
            • Mark Harding
              Mark Harding @markeharding · 2 weeks ago
              Owner

              @var attributes

            • Guy Thouret @gthouret changed this line in version 9 of the diff 2 weeks ago

              changed this line in version 9 of the diff

            • Please register or sign in to reply
          • Mark Harding
            Mark Harding @markeharding · 2 weeks ago
            Owner

            Updating an image failed to propagate with https://chore-616-propogate-props.minds.io/newsfeed/1013879218351640584. Also updating the activity did not update the original post.

            Edited by Mark Harding 2 weeks ago
          • Mark Harding
            Mark Harding @markeharding · 2 weeks ago
            Owner

            Blog also did not propagate at https://chore-616-propogate-props.minds.io/newsfeed/1013879964241498117

          • Guy Thouret @gthouret unmarked as a Work In Progress 2 weeks ago

            unmarked as a Work In Progress

          • Mark Harding
            Mark Harding @markeharding · 2 weeks ago
            Owner

            Video also did not propograte at https://chore-616-propogate-props.minds.io/newsfeed/1013891062705426434 & https://chore-616-propogate-props.minds.io/media/1013890339804549122

          • Mark Harding @markeharding added MR::Requires Changes scoped label and automatically removed MR::Awaiting Review label 2 weeks ago

            added MR::Requires Changes scoped label and automatically removed MR::Awaiting Review label

          • Guy Thouret @gthouret added 1 commit 2 weeks ago

            added 1 commit

            • a60771f1 - (fix) Cast values to integers in Feeds PropogateProperties delegate - #616 (closed)

            Compare with previous version

          • Guy Thouret @gthouret added 2 commits 2 weeks ago

            added 2 commits

            • 1d3fc8d7 - (fix) Fix up the pass by reference logic that was still in place; Add...
            • 8e3fcffe - (chore) Add doc blocks for Propagator\Properties - #616 (closed)

            Compare with previous version

          • Guy Thouret @gthouret added 2 commits 2 weeks ago

            added 2 commits

            • 5576dd6c - (fix) Propogator\Properties abstract should use arrays for actable type and subtype - #616 (closed)
            • 3b4af0b0 - (fix) Correctly propogate media (image|video) titles and hook into newsfeed...

            Compare with previous version

          • Guy Thouret @gthouret added 2 commits 2 weeks ago

            added 2 commits

            • 0c32d3c0 - (fix) Test regression from Media PropagateProperties delegate changes - #616 (closed)
            • 856be50b - (fix) Add missing tests for Media PropagateProperties delegate fromActivity - #616 (closed)

            Compare with previous version

          • Guy Thouret @gthouret added 1 commit 2 weeks ago

            added 1 commit

            • de43ea75 - (fix) Linter for Entities\Propagator\Properties - #616 (closed)

            Compare with previous version

          • Guy Thouret @gthouret added 1 commit 1 week ago

            added 1 commit

            • 427a4422 - (chore) Move functionality from Feeds\AttachmentPaywallDelegate to...

            Compare with previous version

          • Guy Thouret @gthouret changed title from Propogate Properties when entities get updated to Propagate Properties when entities get updated 1 week ago

            changed title from Propogate Properties when entities get updated to Propagate Properties when entities get updated

          • Guy Thouret @gthouret added MR::Awaiting Review scoped label and automatically removed MR::Requires Changes label 5 days ago

            added MR::Awaiting Review scoped label and automatically removed MR::Requires Changes label

          • Mark Harding @markeharding mentioned in issue front#1889 4 days ago

            mentioned in issue front#1889

          • Mark Harding
            Mark Harding @markeharding · 4 days ago
            Owner

            Pending front#1889 being completed @gthouret

          • Guy Thouret @gthouret added 1 commit 3 days ago

            added 1 commit

            • 00a636a8 - (fix) Blog activity was not being created for 'loggedIn' access setting - Related to front#1889

            Compare with previous version

          • Guy Thouret @gthouret added 1 commit 3 days ago

            added 1 commit

            • 31cffea2 - (fix) Linter wanted strictness in blog api change - Related to front#1889

            Compare with previous version

          • Mark Harding
            Mark Harding @markeharding · 16 minutes ago
            Owner

            Screen_Shot_2019-09-16_at_8.05.37_AM

            Screen_Shot_2019-09-16_at_8.05.32_AM

            Still getting duplicated title/message... This duplicated after I saved the original.

            Edited by Mark Harding 16 minutes ago
          • Mark Harding @markeharding added MR::Requires Changes scoped label and automatically removed MR::Awaiting Review label 16 minutes ago

            added MR::Requires Changes scoped label and automatically removed MR::Awaiting Review label

          • You're only seeing other activity in the feed. To add a comment, switch to one of the following options.
          Please register or sign in to reply
          0 Assignees
          None
          Assign to
          None
          Milestone
          None
          Assign milestone
          None
          Time tracking
          No estimate or time spent
          3
          Labels
          MR::Requires Changes Priority::2 - Normal Squad::Blue
          Assign labels
          • View project labels
          Lock merge request
          Unlocked
          9
          9 participants
          user avatar
          Brian Hatchet
          user avatar
          Ben Hayward
          user avatar
          Emiliano Balbuena
          user avatar
          Martin Santangelo
          user avatar
          Rami Albatal
          user avatar
          Mark Harding
          user avatar
          Marcelo Rivera
          Reference: minds/engine!303