Page MenuHomeSoftware Heritage

rename add forge now status from 'denied' to 'unsuccessful'
ClosedPublic

Authored by bchauvet on Aug 16 2022, 4:20 PM.

Diff Detail

Repository
rDWAPPS Web applications
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Build is green

Patch application report for D8246 (id=29763)

Rebasing onto 27e448ca12...

Current branch diff-target is up to date.
Changes applied before test
commit 9733d313a0750d8dd90df8d4d49d92010ebc2367
Author: Benoit Chauvet <contact@benoitchauvet.com>
Date:   Tue Aug 16 16:16:58 2022 +0200

    rename add forge now status from denied to unsuccessful

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1965/ for more details.

Shouldn't the migration update existing DENIED values to UNSUCCESSFUL?

anlambert added a subscriber: anlambert.

Looks good to me but we should add a new test in that file checking the migration can be successfully applied.
I would go for that scenario: before applying migration, check the unsucessfull status cannot be set, after applying migration, check the status can be set.

swh/web/add_forge_now/migrations/0007_auto_20220816_1413.py
1 ↗(On Diff #29763)

The file should be renamed to 0007_rename_denied_request_status.py to know what about that migration is.

This revision now requires changes to proceed.Aug 16 2022, 4:38 PM

Shouldn't the migration update existing DENIED values to UNSUCCESSFUL?

We do not have requests in production database with such statuses so I do not think we need to add such processing
but in case @bchauvet want to implement it, he can inspire from that migration.

Build has FAILED

Patch application report for D8246 (id=29765)

Rebasing onto 27e448ca12...

Current branch diff-target is up to date.
Changes applied before test
commit 159cf2e381903cc79203cffd62ee7b02282fe886
Author: Benoit Chauvet <contact@benoitchauvet.com>
Date:   Tue Aug 16 16:16:58 2022 +0200

    rename add forge now status from denied to unsuccessful

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1967/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1967/console

Build has FAILED

Patch application report for D8246 (id=29773)

Rebasing onto 27e448ca12...

Current branch diff-target is up to date.
Changes applied before test
commit fa14dde554f658ad96e482a4149d4e8ef50a2054
Author: Benoit Chauvet <contact@benoitchauvet.com>
Date:   Wed Aug 17 10:09:51 2022 +0200

    rename add forge now status from denied to unsuccessful

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1968/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1968/console

swh/web/tests/add_forge_now/test_migration.py
181

Use RequestStatus.UNSUCCESSFUL.name here to set the value and fix the test.

199

same here

bchauvet added inline comments.
swh/web/tests/add_forge_now/test_migration.py
181

great, I had the right feeling :)

Build is green

Patch application report for D8246 (id=29774)

Rebasing onto 27e448ca12...

Current branch diff-target is up to date.
Changes applied before test
commit ee9f6ab31199a62bd404fb0fa64e15500e2bbdeb
Author: Benoit Chauvet <contact@benoitchauvet.com>
Date:   Wed Aug 17 10:09:51 2022 +0200

    rename add forge now status from denied to unsuccessful

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1970/ for more details.

bchauvet marked an inline comment as done.

added a missing transition from 'feedback to handle' to 'unsuccessful'

Build has FAILED

Patch application report for D8246 (id=29777)

Rebasing onto 27e448ca12...

Current branch diff-target is up to date.
Changes applied before test
commit 50a61fc311eb54bcaf3aa077388446ce75d6623b
Author: Benoit Chauvet <contact@benoitchauvet.com>
Date:   Wed Aug 17 10:09:51 2022 +0200

    rename add forge now status from denied to unsuccessful

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1971/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1971/console

test status changes from feedback_to_handle

Build has FAILED

Patch application report for D8246 (id=29785)

Rebasing onto 27e448ca12...

Current branch diff-target is up to date.
Changes applied before test
commit b60c0e518a421d464b453ca0d3fd36017bd98425
Author: Benoit Chauvet <contact@benoitchauvet.com>
Date:   Wed Aug 17 10:09:51 2022 +0200

    rename add forge now status from denied to unsuccessful

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1974/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1974/console

swh/web/tests/add_forge_now/test_models.py
25

RequestStatus.UNSUCCESSFUL must be added at the end of the list for the test to succeed.

Build is green

Patch application report for D8246 (id=29787)

Rebasing onto 27e448ca12...

Current branch diff-target is up to date.
Changes applied before test
commit b6ddca59dd5ed801745b47b6e92b6ef1ffda65f5
Author: Benoit Chauvet <contact@benoitchauvet.com>
Date:   Wed Aug 17 10:09:51 2022 +0200

    rename add forge now status from denied to unsuccessful

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1975/ for more details.

This revision is now accepted and ready to land.Aug 17 2022, 4:20 PM