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

[WOC] Wagtail - Developer & Funder Guides #2130

Merged
merged 10 commits into from
Dec 4, 2018
Merged

[WOC] Wagtail - Developer & Funder Guides #2130

merged 10 commits into from
Dec 4, 2018

Conversation

pinkiebell
Copy link
Contributor

To import initial pages:

docker-compose exec web bash
cd app && cat cms/fixtures/guides.json|./manage.py loaddata --format json -

NOTE: The fixture data does not include media files, so the pages needs to be fixed.

Fixes #1860

@pinkiebell pinkiebell mentioned this pull request Aug 31, 2018
12 tasks
@codecov
Copy link

codecov bot commented Aug 31, 2018

Codecov Report

Merging #2130 into master will increase coverage by 0.1%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #2130     +/-   ##
=========================================
+ Coverage   29.79%   29.89%   +0.1%     
=========================================
  Files         180      181      +1     
  Lines       13722    13742     +20     
  Branches     1840     1840             
=========================================
+ Hits         4088     4108     +20     
  Misses       9496     9496             
  Partials      138      138
Impacted Files Coverage Δ
app/app/settings.py 80.23% <100%> (+0.15%) ⬆️
app/cms/models.py 100% <100%> (ø)
app/app/urls.py 88% <100%> (+0.76%) ⬆️

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 fcc701c...7699e28. Read the comment docs.

requirements/base.txt Outdated Show resolved Hide resolved
'bounty_requests',

# wagtail
'taggit',
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we include TAGGIT_CASE_INSENSITIVE = env.bool('TAGGIT_CASE_INSENSITIVE', default=True) somewhere in the settings?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good idea

app/cms/models.py Show resolved Hide resolved
app/cms/templates/cms/guide_page.html Outdated Show resolved Hide resolved
@pinkiebell
Copy link
Contributor Author

@mbeacom I resolved your comments 👍

@pinkiebell pinkiebell changed the title WIP: Wagtail - Developer & Funder Guides Wagtail - Developer & Funder Guides Aug 31, 2018
@pinkiebell
Copy link
Contributor Author

Final update. The images must be uploaded & added later on production with wagtail.
You may want to update some paragraphs, some links have FIXME's in their names to indicate that I didn't know the url(s) 😅 .



class GuidePage(Page):
"""Defines the structure for the GuidePage."""
Copy link
Contributor

Choose a reason for hiding this comment

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

@pinkiebell Generally, looks good. Going to throw this on staging.

Random note regarding docstrings/spacing with pep-257/258:

Classes call for a blank line like so:

class Class:
    """Some class docstring."""

    var = 'derp'

Methods:

def derp():
    """Some derp method."""
    var = 'derp'

Some other spacing notes all together:

from __future__ import braces


class Derp:
    """Derp docstring."""

    def derp(self):
        """Some Derp.derp class method docstring."""
        var = 'derp'

    def derp2(self):
        """Some Derp.derp2 class method docstring."""
        var = 'derp2'


def derp(self):
    """Some derp method docstring."""
    var = 'derp'


def derp2(self):
    """Some derp2 method docstring."""
    var = 'derp2'

Also, when possible, we follow the google style pep-257/258 docstrings:

def derp(dingo, dance='tango'):
    """Handle processing a dingo based on the provided dance.

    Args:
        dingo (dashboard.Derp): The Derp object.
        dance (str): The dance type.  Defaults to: tango.

    Returns:
        bool: Whether or not the Derp performs the specified dance.

    """
    return dance in dingo.known_dances

@mbeacom
Copy link
Contributor

mbeacom commented Sep 6, 2018

screenshot 2018-09-06 13 00 14

Seeing this on staging at: https://stage.gitcoin.co/cms/pages/

Is there something I should adjust here? Migrations have ran successfully and fixtures have been imported.

@pinkiebell
Copy link
Contributor Author

@mbeacom
Does a (super)user with id=1 exist on staging?
I tried that locally with a clean setup and apart from importing the fixtures and adding the superuser - I had no problems 🤔

@mbeacom
Copy link
Contributor

mbeacom commented Sep 6, 2018

@pinkiebell We have a pk=1 user, but it's not a superuser... That was while I was logged in with my standard user (mbeacom) with is_superuser/is_staff=True

@pinkiebell
Copy link
Contributor Author

@mbeacon
Can you try that out locally?

@pinkiebell
Copy link
Contributor Author

@mbeacom
I can't reproduce the error on my site :/

@pinkiebell
Copy link
Contributor Author

Quick rebase. I tried both with a purged web_pgdata & and with a backup against latest master.
I have no errors and the exception value is confusing.
See https://github.com/wagtail/wagtail/blob/master/wagtail/core/models.py#L290
UserProfile has no 'go_live_at' field, thats right, Page has.
So, the fixture data is fine - there is no UserProfile in the fixture data.
Clueless right now ;/

@PixelantDesign
Copy link
Contributor

@pinkiebell any updates on this or questions from your end on @mbeacom comments?

@pinkiebell
Copy link
Contributor Author

@PixelantDesign
Well, we reasoned about that error on slack a bit.
I can't reproduce that locally and can't debug more because I don't know much about the staging environment. 🤔

@pinkiebell
Copy link
Contributor Author

@mbeacom Ready for a spin 👍

@pinkiebell
Copy link
Contributor Author

@mbeacom
I think those errors comes from the wrong content_type defined in the fixtures.
content_type seems to relate to other db models and is expressed as an int; ouch!
That means everything ever created on staging (different db models) will screw up those fixtures.

So that needs to be manually fixed 💥 .

screenshot 2018-11-20 at 15 19 29

@pinkiebell
Copy link
Contributor Author

Ah, and the administration page doesn't help much because pagerevisions are not displayed there;
and these needs to be manually edited in the fixture. (the content_type )

@owocki
Copy link
Contributor

owocki commented Nov 25, 2018

did some QA on this today

  1. i get a server error when i load the fixtures
  2. when i try to save a page i get a 500 screenshot
  3. when i was able to create a page (before the fixtures were loaded) the alignment seemed off (the intro p was left aligned and off to the left)

@pinkiebell looking forward to seeing this work! FWIW, it was working pretty well before i loaded the fixtures..

@pinkiebell
Copy link
Contributor Author

@owocki
Thanks for the QA. The server errors coming from the fixtures.
Actually it is impossible to import those without breaking something.
That is because wagtail handles the data based on the content_type - a integer to the db model I guess. That is always different depending what models/changes you already have or had on your local instance.

I will check the alignment of the header and I guess @mbeacom has to manually inject the data...

@owocki
Copy link
Contributor

owocki commented Nov 26, 2018

@pinkiebell how do you plan to insert the data if not with fixtures? can u have me a SQL dump?

screenshot of the alignment issues here:
screencapture-localhost-8000-cms-pages-3-edit-preview-2018-11-25-19_10_13

@@ -402,6 +405,11 @@
# gitcoinbot
url(settings.GITHUB_EVENT_HOOK_URL, gitcoinbot.views.payload, name='payload'),
url(r'^impersonate/', include('impersonate.urls')),

# wagtail
re_path(r'^cms/', include(wagtailadmin_urls)),
Copy link
Contributor

Choose a reason for hiding this comment

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

@pinkiebell
two questions:

  • is there any public facing index of all of the helpdesk items we can use?
  • is there any way to get the helpdesk items into the sitemap (see sitemaps.py)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@owocki

  1. That would have to be built. It is also possible to redirect to a custom page in the meantime.
  2. Wagtail has a sitemap module; that should work.

@pinkiebell
Copy link
Contributor Author

@pinkiebell how do you plan to insert the data if not with fixtures? can u have me a SQL dump?

A sql dump is useless in this case too.
I edited the fixtures a bit and it should now be possible to preview them,
one workaround could be copy & pasting from the preview to a newly created page 🔨 😓 .

For more background: wagtail/wagtail#1366

@mbeacom mbeacom changed the title Wagtail - Developer & Funder Guides WOC: Wagtail - Developer & Funder Guides Nov 29, 2018
@mbeacom mbeacom changed the title WOC: Wagtail - Developer & Funder Guides [WOC] Wagtail - Developer & Funder Guides Nov 29, 2018
Copy link
Contributor

@owocki owocki left a comment

Choose a reason for hiding this comment

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

@pinkiebell this looks _amazing.. thank you!

@mbeacom ok to 🚢 from your perspective?

@owocki
Copy link
Contributor

owocki commented Dec 4, 2018

hey @pinkiebell im trying to deploy these live now and i keep getting

ValidationError: {'content_type': ['This field cannot be null.']}

error when i try to publish them

screenshot : http://bits.owocki.com/ba98fa2a9be8/Screen%20Shot%202018-12-04%20at%203.03.12%20PM.png

@pinkiebell
Copy link
Contributor Author

@owocki
That is the wagtail problem I mentioned. You have to create a new Guide page and copy the rich text content from the 'broken' existing page(s) to a new one.
Sorry for the hassle but that is something wagtail didn't get right 😅

@pinkiebell
Copy link
Contributor Author

@owocki
And please note that some youtube video links are missing because I didn't know the exact link I had to put there.

@owocki
Copy link
Contributor

owocki commented Dec 5, 2018

@pinkiebell how do i get to the live pages once they're published'. the links all seem to be broken to me - screencast

@owocki
Copy link
Contributor

owocki commented Dec 5, 2018

also note it says "There is no site set up for this location. Pages created here will not be accessible at any URL until a site is associated with this location. Configure a site now." even though i do have a site setup

screenshot

screenshot of site

@owocki owocki mentioned this pull request Dec 5, 2018
3 tasks
@owocki
Copy link
Contributor

owocki commented Dec 5, 2018

@pinkiebell
Copy link
Contributor Author

@owocki
Great! 🎉

@pinkiebell pinkiebell deleted the wagtail branch January 3, 2019 21:21
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.

BUILD Funder & Developer Guides
4 participants