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

Remove restapi #106

Merged
merged 8 commits into from
Jun 18, 2018
Merged

Remove restapi #106

merged 8 commits into from
Jun 18, 2018

Conversation

rodfersou
Copy link
Member

@rodfersou rodfersou commented Jun 14, 2018

No description provided.

@rodfersou rodfersou force-pushed the remove-restapi branch 2 times, most recently from f2410ca to 8f6f29e Compare June 15, 2018 15:03
@idgserpro
Copy link
Member

Why?

@hvelarde
Copy link
Member

@idgserpro
Copy link
Member

Don't forget do document this motivation in the final commit of this PR.

@rodfersou rodfersou force-pushed the remove-restapi branch 3 times, most recently from e3d3b60 to 2f17bd6 Compare June 15, 2018 18:58
Copy link
Member

@hvelarde hvelarde left a comment

Choose a reason for hiding this comment

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

we need an upgrade step to cook static resources.

now = datetime.now()
tzname = datetime.now(tzlocal()).tzname()
selected = datetime.strptime(self.date, AGENDADIARIAFMT)
days = [selected + timedelta(days=i) for i in range(-3, 4)]
Copy link
Member

Choose a reason for hiding this comment

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

range is inefficient in Python 2.

"""Visao padrao da agenda."""

def setup(self):
self._ts = getToolByName(self.context, 'translation_service')
Copy link
Member

Choose a reason for hiding this comment

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

use the API: api.portal.get_tool()

setup.py Outdated
@@ -43,6 +43,7 @@
'Acquisition',
'collective.cover',
'collective.portlet.calendar',
'cssselect',
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't this be a test dependency?

Copy link
Member Author

Choose a reason for hiding this comment

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

not in this case.. the exception raised when start the instance in my tests.

current_language = portal_state.language()
portal_state = getMultiAdapter((self, self.REQUEST),
name=u'plone_portal_state')
current_language = portal_state.language()
Copy link
Member

Choose a reason for hiding this comment

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

use the API: api.portal.get_current_language()


def publishTraverse(self, request, date):
"""Pega a data da agenda diaria."""
self.date = date
Copy link
Member

Choose a reason for hiding this comment

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

we need to check here that date is a valid date.

@rodfersou rodfersou force-pushed the remove-restapi branch 4 times, most recently from e03e1e3 to 68821e0 Compare June 18, 2018 21:51
@hvelarde hvelarde merged commit 2ca89cd into master Jun 18, 2018
@hvelarde hvelarde deleted the remove-restapi branch June 18, 2018 23:40
@@ -7,6 +7,7 @@ extends =
package-name = brasil.gov.agenda
package-extras = [test]
eggs +=
cssselect # XXX: needed by plone.protect
Copy link
Member

Choose a reason for hiding this comment

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

Tem uma explicação melhor sobre isso? Na branch 1.x os testes passam, mas se eu dou checkout para fazer um teste exploratório, não consigo salvar uma agenda.

Copy link
Member

Choose a reason for hiding this comment

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

agora estamos usando uma versão de lxml maior e eles removeram o cssselect de ai.

Copy link
Member

Choose a reason for hiding this comment

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

Sim, o traceback começa no lxml, só achei engraçado os testes passarem. Vou adicionar essa dependência na branch 1.x.

Copy link
Member

Choose a reason for hiding this comment

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

na verdade acho que a dependência tem que ser adicionada no plone.protect.

Copy link
Member

Choose a reason for hiding this comment

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

idgserpro added a commit that referenced this pull request Jun 27, 2018
Ver #106 (comment). Sem essa dependência dá erro no plone.protect devido à versão mais nova de lxml.
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.

3 participants