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

dashboard/views: add module type in ProjectCreateView #1207

Merged
merged 2 commits into from
Sep 8, 2022

Conversation

Rineee
Copy link
Contributor

@Rineee Rineee commented Sep 6, 2022

Needed for external projects, bplans and project containers.

Kind of belongs to liqd/a4-meinberlin#4449

@github-actions
Copy link

github-actions bot commented Sep 6, 2022

Coverage report

Total coverage

Status Category Percentage Covered / Total
🔴 Statements 9.37% 103/1099
🔴 Branches 9.52% 68/714
🔴 Functions 10.39% 37/356
🔴 Lines 15.65% 530/3386

Status of coverage: 🟢 - ok, 🟡 - slightly more than threshold, 🔴 - under the threshold

Report generated by 🧪jest coverage report action from b0b3120

Copy link
Contributor

@fuzzylogic2000 fuzzylogic2000 left a comment

Choose a reason for hiding this comment

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

Looks good!
I am a bit unsure about adding the types to the settings, but I also see no other way to do it. (And let's discuss that in the other PR.)
I also wasn't aware that we overwrite the ProjectCreateView for the mB-Projects, but then still use this one for the external projects and so on. Makes sense, but is also a bit surprising. Maybe we should add a comment here or somewhere, were it's actually used and also what it's for (Adding one-module-projects).
Should we merge right away or do you want to add the test first?

@Rineee
Copy link
Contributor Author

Rineee commented Sep 7, 2022

Looks good! I am a bit unsure about adding the types to the settings, but I also see no other way to do it. (And let's discuss that in the other PR.) I also wasn't aware that we overwrite the ProjectCreateView for the mB-Projects, but then still use this one for the external projects and so on. Makes sense, but is also a bit surprising. Maybe we should add a comment here or somewhere, were it's actually used and also what it's for (Adding one-module-projects). Should we merge right away or do you want to add the test first?

@fuzzylogic2000 I thought, the other possibility would be to just add empty blueprint type for bplans etc (so just type = '' in the defintions of the blueprints for bplan, ext proj, proj container).. then we would not need this and also not need to add those in the settings? But then they would show up empty in django admin.. dont really know, what would you prefer?

@Rineee
Copy link
Contributor Author

Rineee commented Sep 7, 2022

@fuzzylogic2000 Ok, I finally added a test..

Copy link
Contributor

@fuzzylogic2000 fuzzylogic2000 left a comment

Choose a reason for hiding this comment

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

Nice!

@fuzzylogic2000 fuzzylogic2000 merged commit ec91190 into main Sep 8, 2022
@fuzzylogic2000 fuzzylogic2000 deleted the ks-2022-project-create-blueprints branch September 8, 2022 07:41
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.

2 participants