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

Add investments table and partners page #2126

Merged
merged 19 commits into from
May 5, 2021
Merged

Add investments table and partners page #2126

merged 19 commits into from
May 5, 2021

Conversation

sks444
Copy link
Contributor

@sks444 sks444 commented Sep 14, 2020

Fixes ResetNetwork#79

Steps to test this feature:

  • Add Partner Index Page as a child of About page. (similar to Person Index Page)
  • Add partners as child of partnerindex.
  • Go to Investments and create Investments for partners that you created.
  • Visit the partners in frontend by going to about tab. You'll get a table of all partners with their investments made.

TODOs:

  • From Dan's comment: Option to include and require one or more Category Question, for example:
    -- type: Can we use the Category Question "Organization Type"?
    • If I understand correctly this means that we will have a fixed number of fields defined in model and we will have a category question defined for them in the admin. So that we can add any number of options/choices to that.
    • OR, we want to have the functionality of adding any number of category questions(fields) to these pages/models?
      • But for fields to be viewed in the table and for searching/filtering they should be defined in the models and not be dynamically defined via admin.

@sks444 sks444 requested a review from Parbhat September 14, 2020 07:41
)

created_at = models.DateTimeField(auto_now_add=True)
updated_at = models.DateTimeField(auto_now_add=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
updated_at = models.DateTimeField(auto_now_add=True)
updated_at = models.DateTimeField(auto_now=True)

Copy link
Contributor

@Parbhat Parbhat left a comment

Choose a reason for hiding this comment

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

Good first step. @frjo Can we deploy this branch to the testing environment, to get feedback from Dan? It will help to get more inputs about the requirements.

@frjo
Copy link
Contributor

frjo commented Sep 14, 2020

Deploying this to test now.

@frjo
Copy link
Contributor

frjo commented Sep 14, 2020

Redeployed latest changes to test.

@sks444
Copy link
Contributor Author

sks444 commented Sep 14, 2020

Thanks @frjo.

One thing that we need to discuss is the requirements of Category Questions and the feasibility of its implementations. Have mentioned some details in the pr description.

Other than that, we can add search/filter and other functionalities later on when the dataset grows.

@danblah
Copy link
Contributor

danblah commented Sep 15, 2020

This is great. Feedback here: ResetNetwork#79 (comment)

@sks444 sks444 force-pushed the feature/partners-page branch 3 times, most recently from 41c2154 to 159864d Compare September 28, 2020 07:04
@sks444
Copy link
Contributor Author

sks444 commented Sep 28, 2020

@frjo, can we redeploy this to test.

Here are the updates that has been made.

@frjo frjo temporarily deployed to test-opentech-fund September 29, 2020 12:37 Inactive
@frjo
Copy link
Contributor

frjo commented Sep 29, 2020

@sks444 Deployed to test now.

Base automatically changed from master to main January 22, 2021 06:47
@sks444
Copy link
Contributor Author

sks444 commented Mar 22, 2021

@frjo, can we redeploy this to test.

It looks like few of the things failing here, were because of the migration conflict.

@frjo
Copy link
Contributor

frjo commented Mar 22, 2021

@sks444 Redeployed to test now.

@sks444
Copy link
Contributor Author

sks444 commented Mar 22, 2021

Thanks @frjo.

It looks like the data entry created from the old migration is still there in the cms. https://test-apply.opentech.fund/admin/pages/17/ (Our Partners). Now I can not delete/edit/add as it will raise 500.

This problem I have been noticing in wagtail from quite some time now. Pages are cached and remain there even if we unapply the corresponding migrations. So the process is to first delete the pages from wagtail and then unapply the migrations.

@frjo, can we do the following:

We need to delete current Our Partners entry from the admin.

  • python manage.py migrate partner zero
  • Checkout to some other branch, maybe main. In this case, that means deploying main to test or reverting the deployment of current branch.
  • Go ahead and delete Our Partners from cms.
  • redeploy the current branch.

@frjo
Copy link
Contributor

frjo commented Mar 22, 2021

@sks444 Done, hope I managed to fix it.

@sks444
Copy link
Contributor Author

sks444 commented Mar 22, 2021

Thank you so much for the effort @frjo.

I still see Our Partners at https://test-apply.opentech.fund/admin/pages/17/

Can you try once again to delete that when you're on other branch where partners code is not deployed. And then redeploy the partners branch.

Screenshot 2021-03-22 at 7 29 55 PM

This is cached page I believe. So will have to delete it from where the current branch code is not deployed. And then redeploy the current branch.

Let me know if that doesn't work out. I am also not 100% sure on this, I just do this fix when I face similar kind of problem locally sometime.

@frjo
Copy link
Contributor

frjo commented Mar 22, 2021

@sks444 Now that page is gone as well.

@frjo frjo added the Type: Minor Minor change, used in release drafter label Apr 30, 2021
@codecov-commenter
Copy link

codecov-commenter commented May 5, 2021

Codecov Report

Merging #2126 (fb8eeef) into main (edee89a) will decrease coverage by 0.26%.
The diff coverage is 60.43%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2126      +/-   ##
==========================================
- Coverage   82.11%   81.84%   -0.27%     
==========================================
  Files         607      614       +7     
  Lines       21849    22126     +277     
==========================================
+ Hits        17942    18110     +168     
- Misses       3907     4016     +109     
Impacted Files Coverage Δ
hypha/apply/funds/tables.py 82.41% <0.00%> (ø)
hypha/public/partner/apps.py 0.00% <0.00%> (ø)
hypha/settings/base.py 70.24% <ø> (ø)
hypha/public/partner/admin_view.py 29.03% <29.03%> (ø)
hypha/public/partner/models.py 58.67% <58.67%> (ø)
hypha/public/partner/tables.py 60.00% <60.00%> (ø)
hypha/public/partner/views.py 75.00% <75.00%> (ø)
hypha/public/partner/admin.py 100.00% <100.00%> (ø)
...ns/0001_add_investments_table_and_partners_page.py 100.00% <100.00%> (ø)
hypha/public/urls.py 100.00% <100.00%> (ø)
... and 7 more

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 edee89a...fb8eeef. Read the comment docs.

@frjo frjo temporarily deployed to test-hypha-app May 5, 2021 10:11 Inactive
@frjo frjo added the Type: Feature This is something new (not an enhancement of an existing thing). label May 5, 2021
@frjo frjo merged commit 423fe8d into main May 5, 2021
@danblah
Copy link
Contributor

danblah commented Sep 8, 2021

@frjo and @sks444, why don't I have the option to create an about child page on our production? Did I fail to fully test/confirm this?

@frjo frjo deleted the feature/partners-page branch March 18, 2022 07:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature This is something new (not an enhancement of an existing thing). Type: Minor Minor change, used in release drafter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

We want a "Partners" page that looks like Luminate's
5 participants