Import budgeting projects into accountability results by sinaeftekhar · Pull Request #9604 · decidim/decidim

sinaeftekhar · 2022-07-25T09:15:48Z

🎩 What? Why?

A normal proceeding order in a PB process is:

Collect and work on ideas ("proposals")
Arrange the PB voting ("projects")
Follow-up on the progress of the projects that won ("results")

Right now we can import accepted proposals to projects.
But, we cannot import "selected" projects to results.

This is a problem for admins who prefer not to waste their time doing copy-paste work.
We have added a similar importing feature in the accountability component as we have right now in the budgets component.

The import considers the following:

Copy the title and description from the project
Map the result to the same category and scope that the project had (or the budget's scope if the project does not have a scope defined)
Add the first "default" status to the new results
Add a "default" progress value from the first status or zero if no statuses are available

Above, the "default" status refers to the first status ordered by their progress percentages.

📌 Related Issues

Related to https://meta.decidim.org/processes/roadmap/f/122/proposals/16982

Testing

No prerequisite is needed to run the tests. Run the tests, as if you would normally.

📷 Screenshots

Navigate to the your Accountability component, and click on "Import Projects to Results" link (as shown below)

Now, select the origin component, from which you want to import the selected projects, select "Import all selected projects" and click Import:

Your project is now added to the list with a success message:

♥️ Thank you!

The imports works with some exceptions: - The system does not check if the record is already exist. - The import don't show how many records are being imported.

Add js functionality, so the number of items to be added is being shown on as the user select the original component for import.

move some shared methods to form section and remove the redundants.

ahukkanen

Overall great work for the first feature PR! Everything works as expected when testing it in the development app.

Regarding the code, I've left some refactoring comments below, can you please go through.

I've also found one bug that the message how many projects are available is not visible if I submit the form and it had errors:

Also, this feels kind of weird as a "success" message:

So can we either
a) change the message to an error message showing something like "There were no projects to import within the selected component"
OR
b) prevent submitting the form in case there are no projects altogether

decidim-accountability/config/locales/en.yml

decidim-accountability/spec/commands/admin/import_projects_to_accountability_spec.rb

decidim-accountability/spec/forms/admin/result_import_projects_form_spec.rb

decidim-accountability/spec/permissions/decidim/accountability/admin/permissions_spec.rb

decidim-accountability/spec/system/admin/admin_imports_projects_to_accountability_spec.rb

ahukkanen · 2022-07-28T08:37:04Z

I found this old bug report regarding the proposal imports (import proposals between components): #7608.

I think this is also a relevant point regarding this PR that we should consider here.

So, we still need to do the following changes here:

We need to move the import processing to a background job and when the background job is successfully started, show a success message such as "Import processing has stated. You will receive an email after it has completed."
Once the import processing has successfully completed, send an email to the user who started it with the information on how many results were imported and to which component (with a link to the component admin panel listing the results).

We have similar functionality already in Decidim where you can take inspiration from, such as the CSV import within the same accountability component.

The other point about handling long running jobs better mentioned at #7608 is a separate issue and we need to tackle that later on. Right now, just follow what is already being done in other imports that are background processed.

ahukkanen

It's starting to be pretty close to completed.

A couple of further cleanup efforts + one bugfix still needed I noticed when going through.

...im-accountability/app/controllers/decidim/accountability/admin/projects_import_controller.rb

decidim-accountability/app/forms/decidim/accountability/admin/result_import_projects_form.rb

decidim-accountability/app/packs/src/decidim/accountability/admin/imports.js

decidim-accountability/app/packs/entrypoints/decidim_accountability_admin_imports.js

...ountability/spec/controllers/decidim/accountability/admin/projects_import_controller_spec.rb

decidim-accountability/spec/mailers/import_projects_mailer_spec.rb

decidim-accountability/spec/jobs/decidim/accountability/admin/import_projects_job_spec.rb

ahukkanen

LGTM!

Before merging, we need to go it through and demo it for @decidim/product before we merge it to the core to see if they are happy with the changes.

carolromero · 2022-08-31T09:58:12Z

Thanks for the demo @sinaeftekhar! I would only suggest grouping the import options (csv, budgets) in a single drop-down button. Otherwise, looks good to me.

sinaeftekhar · 2022-09-01T09:15:52Z

Thanks for the demo @sinaeftekhar! I would only suggest grouping the import options (csv, budgets) in a single drop-down button. Otherwise, looks good to me.

Thanks @carolromero for the comment. The suggestion has been applied, and is now ready for the review

ahukkanen

Great!

Just one more request regarding the import dropdown positionning and then I think this is done.

decidim-accountability/app/views/decidim/accountability/admin/results/index.html.erb

ahukkanen

Great work!

Before merging this, could you @carolromero also confirm that this is now good to go from product standpoint? This is what the requested import dropdown now looks like after the latest changes:

carolromero · 2022-09-02T10:46:38Z

@ahukkanen looks great, green light to merge!

* develop: Add additional details in HTTP 500 error page message (#9762) Publication date field for blog articles (#9757) Import budgeting projects into accountability results (#9604)

* develop: Fix doorkeeper initialization after 5.6.0 release (#9785) Add additional details in HTTP 500 error page message (#9762) Publication date field for blog articles (#9757) Import budgeting projects into accountability results (#9604) Redesign: flash messages (#9774)

* develop: Fix doorkeeper initialization after 5.6.0 release (#9785) Add additional details in HTTP 500 error page message (#9762) Publication date field for blog articles (#9757) Import budgeting projects into accountability results (#9604)

* Import projects without checkpoints The imports works with some exceptions: - The system does not check if the record is already exist. - The import don't show how many records are being imported. * Fix bugs in adding already existing projects twice * Complete import functionality Add js functionality, so the number of items to be added is being shown on as the user select the original component for import. * Add permission to imports * revise en locale for import message * Remove text color when there are projects * Fix a bug in command with current_component * Remove unused methods from the controller * Add tests for commands * Complete tests for commands * Mark unnecessary methods to be removed later * Complete tests for ProjectImportController * Refactor ImportProjectsToResults command move some shared methods to form section and remove the redundants. * Fix some introduced bugs from previous refactoring * Complete form tests * Add permission specs for the feature * Refactor commands; remove extra spaces * Add system test * Show import link only in main page * Fix linting violations * Fix lint violation in imports.js file * Add error when there are no projects to import * Fix lint violations * Address reviewers' comments These changes have been applied: - Redundant testing regrading for the permissions were removed - Messages for the translation has been updated - Extra comments were removed. * Add mailer job to the Imports The import is now being handled with a job, and the user will be informed once the projects are imported. * Fix a bug regarding the translation * Add job to import task The followings are being changes/edited: - The tasks related to import projects moves to the job - Mailer task added to inform the user once the task finished - Fix violated tests, and added required tests - Refactored some parts, with respect to the concept of job * Fix a linting error * Remove unused locales * Fix issues mentioned by reviewer's * Revert package-lock.json * Fix the last set of comments * Fix a typo * Fix errors in pull request failure * Fix a bug in a spec * Resolve issues mentioned by reviewers * Wrap up the import buttons into an 'Import button' * Change the position of import button in the view

sinaeftekhar added 19 commits July 19, 2022 11:51

Import projects without checkpoints

ddac18f

The imports works with some exceptions: - The system does not check if the record is already exist. - The import don't show how many records are being imported.

Fix bugs in adding already existing projects twice

efb63b6

Complete import functionality

43f7858

Add js functionality, so the number of items to be added is being shown on as the user select the original component for import.

Add permission to imports

f09bea3

revise en locale for import message

e04f6cf

Remove text color when there are projects

4c72c7a

Fix a bug in command with current_component

1cd4a39

Remove unused methods from the controller

a728d0a

Add tests for commands

05e3498

Complete tests for commands

5573d42

Mark unnecessary methods to be removed later

4532a4c

Complete tests for ProjectImportController

4f028b4

Refactor ImportProjectsToResults command

50714b3

move some shared methods to form section and remove the redundants.

Fix some introduced bugs from previous refactoring

759674f

Complete form tests

169f1f0

Add permission specs for the feature

222c88d

Refactor commands; remove extra spaces

4f58830

Add system test

410ef6a

Merge branch 'develop' into feature/import-budgets-from-accountability

eb96c06

ahukkanen changed the title ~~Feature/import budgets from accountability~~ Import budgeting projects into accountability results Jul 25, 2022

ahukkanen added type: feature module: accountability labels Jul 25, 2022

sinaeftekhar added 3 commits July 25, 2022 12:59

Show import link only in main page

c0b3fea

Fix linting violations

9f2b117

Fix lint violation in imports.js file

dba390c

ahukkanen requested changes Jul 26, 2022

View reviewed changes

sinaeftekhar added 3 commits August 2, 2022 12:07

Add error when there are no projects to import

b0a81f5

Merge branch 'develop' into feature/import-budgets-from-accountability

101f679

Fix lint violations

a3ade34

sinaeftekhar marked this pull request as draft August 9, 2022 07:57

sinaeftekhar added 5 commits August 9, 2022 15:28

Fix the last set of comments

3c3d56d

Merge branch 'develop' into feature/import-budgets-from-accountability

e271690

Fix a typo

7e72ed6

Fix errors in pull request failure

bef653e

Fix a bug in a spec

2fb5490

sinaeftekhar marked this pull request as ready for review August 9, 2022 13:56

sinaeftekhar marked this pull request as draft August 9, 2022 15:20

ahukkanen requested changes Aug 11, 2022

View reviewed changes

Resolve issues mentioned by reviewers

9e45a22

sinaeftekhar marked this pull request as ready for review August 11, 2022 14:51

ahukkanen previously approved these changes Aug 11, 2022

View reviewed changes

sinaeftekhar added 2 commits September 1, 2022 11:51

Merge branch 'develop' into feature/import-budgets-from-accountability

a7f9fff

Wrap up the import buttons into an 'Import button'

405c930

sinaeftekhar dismissed ahukkanen’s stale review via 405c930 September 1, 2022 09:13

sinaeftekhar requested a review from ahukkanen September 1, 2022 12:24

ahukkanen requested changes Sep 2, 2022

View reviewed changes

decidim-accountability/app/views/decidim/accountability/admin/results/index.html.erb Show resolved Hide resolved

Change the position of import button in the view

8a4028f

sinaeftekhar requested a review from ahukkanen September 2, 2022 09:44

ahukkanen approved these changes Sep 2, 2022

View reviewed changes

ahukkanen merged commit a72b605 into decidim:develop Sep 2, 2022

ahukkanen deleted the feature/import-budgets-from-accountability branch September 2, 2022 10:47

alecslupu mentioned this pull request Jan 23, 2025

Fix translation issue on import projects mailer #13894

Merged

Uh oh!

Conversation

sinaeftekhar commented Jul 25, 2022

🎩 What? Why?

📌 Related Issues

Testing

📷 Screenshots

Uh oh!

ahukkanen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ahukkanen commented Jul 28, 2022

Uh oh!

ahukkanen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ahukkanen left a comment

Choose a reason for hiding this comment

Uh oh!

carolromero commented Aug 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sinaeftekhar commented Sep 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ahukkanen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ahukkanen left a comment

Choose a reason for hiding this comment

Uh oh!

carolromero commented Sep 2, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

carolromero commented Aug 31, 2022 •

edited

Loading

sinaeftekhar commented Sep 1, 2022 •

edited

Loading