Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dependency/dependant tracking #426

Merged
merged 10 commits into from
Apr 8, 2021
Merged

Conversation

giggsey
Copy link
Contributor

@giggsey giggsey commented Mar 11, 2021

Show package dependencies in detail view, linking to other packages known in the same organisation.

image

Allow searching for other packages that depend on this package (count is shown on the details view):

image

Fixes #175

src/Entity/Organization/Package/Link.php Outdated Show resolved Hide resolved
src/Query/User/PackageQuery.php Outdated Show resolved Hide resolved
src/Query/User/PackageQuery/DbalPackageQuery.php Outdated Show resolved Hide resolved
@giggsey
Copy link
Contributor Author

giggsey commented Mar 11, 2021

Plenty to do on this, but I want to get some feedback on the structure and the future UI

Annoyingly, my thinking is pretty much the same as Packagist. Showing the actual dependencies above the README. Then maybe a separate package list on a separate page, linked under Actions.

Maybe the dependant list is a special search (depends:giggsey/locale)

@akondas
Copy link
Member

akondas commented Mar 16, 2021

  1. Wow, nice work 👍
  2. Would you be able to add a screenshot of what it looks like?
  3. Ok, now I understand why a separate table. I wonder how it will affect the performance with a large number of packages, but postgres should handle it, I think you can go ahead and continue 👍

@giggsey
Copy link
Contributor Author

giggsey commented Mar 16, 2021

I need ideas for the design. At the moment, it's just using flexbox with wrapped columns.

A 'full' package containing all types:

image

A package with minimal data:

image

(ignore the duplicate require + requires, I haven't done the purging yet)

@giggsey
Copy link
Contributor Author

giggsey commented Mar 31, 2021

Now:
image

@giggsey
Copy link
Contributor Author

giggsey commented Mar 31, 2021

I'm not that familiar with doctrine, and it's complaining about the schema not matching the mapping files.

Any ideas @akondas?

Apart from that, the tests should be passing, and it should be ready for review. I've updated the top comment with the latest screenshots/features.

@akondas
Copy link
Member

akondas commented Apr 1, 2021

So, you can run bin/console d:m:diff to check what's changed. From your branch:

    public function up(Schema $schema) : void
    {
        // this up() migration is auto-generated, please modify it to your needs
        $this->addSql('ALTER TABLE organization_package_link ADD CONSTRAINT FK_4A06082932C8A3DE FOREIGN KEY (organization_id) REFERENCES organization (id) NOT DEFERRABLE INITIALLY IMMEDIATE');
        $this->addSql('CREATE INDEX IDX_4A06082932C8A3DE ON organization_package_link (organization_id)');
    }

    public function down(Schema $schema) : void
    {
        // this down() migration is auto-generated, please modify it to your needs
        $this->addSql('ALTER TABLE organization_package_link DROP CONSTRAINT FK_4A06082932C8A3DE');
        $this->addSql('DROP INDEX IDX_4A06082932C8A3DE');
    }

Besides, the whole thing looks very good, when you're done, let me know, I'll click through it and we'll make a new release 🎉

@giggsey
Copy link
Contributor Author

giggsey commented Apr 1, 2021

Migration now failing due to:

[notice] Migrating up to Buddy\Repman\Migrations\Version20210401061218
[warning] Migration Buddy\Repman\Migrations\Version20200720112146 was executed but did not result in any SQL statements.
[error] Migration Buddy\Repman\Migrations\Version20210401061218 failed during Execution. Error: "An exception occurred while executing 'ALTER TABLE organization_package_link ADD CONSTRAINT FK_4A06082932C8A3DE FOREIGN KEY (organization_id) REFERENCES organization (id) NOT DEFERRABLE INITIALLY IMMEDIATE':

However, this file doesn't exist?

@akondas
Copy link
Member

akondas commented Apr 1, 2021

Hey @giggsey it was my fault, sorry. When I debugged the previous pipeline, I forgot to remove the generated migration. Anyway, there are still some differences:

$this->addSql('ALTER INDEX link_organization_id_idx RENAME TO IDX_4A06082932C8A3DE');

@giggsey
Copy link
Contributor Author

giggsey commented Apr 1, 2021

Thanks @akondas.

That index is currently generated via:

        $this->addSql('CREATE INDEX link_organization_id_idx ON organization_package_link (organization_id)');

I can rename it there, but how does that automatic name get generated when it's fine for the 'link_package_id_idx' index?

@akondas
Copy link
Member

akondas commented Apr 1, 2021

As far as I remember, this name is generated by doctrine, based on mappings:

    /**
     * @ORM\ManyToOne(targetEntity="Buddy\Repman\Entity\Organization")
     * @ORM\JoinColumn(nullable=false)
     */

Maybe you had an older version, or you made it out of your finger? it's hard for me to figure out why this name is wrong ... 🤔

@codecov
Copy link

codecov bot commented Apr 1, 2021

Codecov Report

Merging #426 (8fa3e95) into master (507a4bb) will decrease coverage by 0.04%.
The diff coverage is 97.16%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #426      +/-   ##
============================================
- Coverage     99.38%   99.34%   -0.05%     
- Complexity     1864     1891      +27     
============================================
  Files           297      298       +1     
  Lines          5873     5969      +96     
============================================
+ Hits           5837     5930      +93     
- Misses           36       39       +3     
Impacted Files Coverage Δ Complexity Δ
src/Query/User/PackageQuery/Filter.php 95.83% <83.33%> (-4.17%) 10.00 <4.00> (+4.00) ⬇️
src/Entity/Organization/Package/Link.php 91.30% <91.30%> (ø) 9.00 <9.00> (?)
src/Controller/OrganizationController.php 100.00% <100.00%> (ø) 44.00 <0.00> (+2.00)
src/Entity/Organization/Package.php 100.00% <100.00%> (ø) 43.00 <7.00> (+4.00)
src/Query/User/PackageQuery/DbalPackageQuery.php 100.00% <100.00%> (ø) 35.00 <2.00> (+4.00)
...ackageSynchronizer/ComposerPackageSynchronizer.php 97.81% <100.00%> (+0.35%) 39.00 <0.00> (+4.00)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 507a4bb...8fa3e95. Read the comment docs.

@giggsey giggsey marked this pull request as ready for review April 1, 2021 14:04
@akondas akondas requested review from akondas and karniv00l April 2, 2021 17:41
Copy link
Member

@akondas akondas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested the whole thing, the user experience is very positive. There is still the question of optimizing the number of database queries to consider.
Great work, and thank you @giggsey 🎖️

@akondas akondas merged commit f4307ba into repman-io:master Apr 8, 2021
@giggsey giggsey deleted the dependencies branch April 8, 2021 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Package View Improvements
3 participants