Warn if lib32- variant is missing when adding a group #129
Conversation
When adding a new group, warn if the lib32 variant does not exists via a flashcard. Closes: archlinux#120
|
Do we want to consider adding a link the flash message to directly create a new AVG. |
jelly
force-pushed
the
warn_missing_lib32
branch
from
ba6c01c
to
d2644a1
Compare
tracker/view/add.py
Outdated
| if not Package.query.filter(Package.name == lib32pkg).first(): | ||
| continue | ||
|
|
||
| if CVEGroupPackage.query.filter(CVEGroupPackage.pkgname == lib32pkg, CVEGroupPackage.group == group).first(): |
There was a problem hiding this comment.
can we make this algorithm in a way that we need to issue the queries once for all matches and not in a loop? optimally it looks like a single query can retreive all information we need to iterate over
There was a problem hiding this comment.
I've just realized that we are creating the group here, so we don't really have to query the db! We can just query group.packages and check if it contains a lib32 package, we only need to verify if a variant exists in the repo however.
tracker/view/add.py
Outdated
| @@ -199,4 +200,21 @@ def add_group(): | |||
|
|
|||
| db.session.commit() | |||
| flash('Added {}'.format(group.name)) | |||
| missing_lib32_variant(pkgnames, group) | |||
There was a problem hiding this comment.
This doesn't quite say what it does, either we have a get_missing... and loop+flash here or we name it like flash_missing... or such. missing_lib32_variant itself doesn't say much what it actually does.
test/test_group.py
Outdated
|
|
||
| @create_package(name='foo', version='1.2.3-4') | ||
| @logged_in | ||
| def test_add_group_mising_lib32_invalid(db, client): |
test/test_group.py
Outdated
| @create_package(name='foo', version='1.2.3-4') | ||
| @create_package(name='lib32-foo', version='1.2.3-4') | ||
| @logged_in | ||
| def test_add_group_mising_lib32(db, client): |
| @@ -248,6 +248,37 @@ def test_add_group_with_dot_in_pkgrel(db, client): | |||
| set_and_assert_group_data(db, client, url_for('tracker.add_group'), affected='1.2-3.4') | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
i would love to see a test as well that already has the lib32 variant and adds the non-lib32 one later. maybe that case should be detected and flashed as well 😺
There was a problem hiding this comment.
Fixed, but the code is rather deeply nested, maybe some lambda/filter/map logic can be used to make it nicer.
jelly
force-pushed
the
warn_missing_lib32
branch
from
d2644a1
to
6a7fc65
Compare
|
|
||
|
|
||
| def missing_lib32_variant(pkgnames, group): | ||
| for pkgname in pkgnames: |
There was a problem hiding this comment.
I tested this locally and the behavior is correct. I wanted to make sure that we don't add a "there's a lib32 package added but this still flashes the warning" behavior. Could we add a test for it?
There was a problem hiding this comment.
Good idea! I think test_add_group_missing_lib32_invaid handles this case, since the lib32-foo pakage is not created.
Check if lib32-curl misses curl and if adding curl misses lib32-curl.
jelly
force-pushed
the
warn_missing_lib32
branch
from
6a7fc65
to
e8c6921
Compare
|
This is a year old. Do we want to pick this up, @jelly ? |
When adding a new group, warn if the lib32 variant does not exists via a
flashcard.
Closes: #120