Skip to content
This repository was archived by the owner on Apr 29, 2022. It is now read-only.

Add speaker edit view (fix #932) #934

Merged
merged 3 commits into from
May 8, 2019
Merged

Conversation

umgelurgel
Copy link
Contributor

@umgelurgel umgelurgel commented May 7, 2019

Fixes #932

@umgelurgel umgelurgel requested a review from artcz May 7, 2019 14:31
)

if request.method == 'POST':
speaker_form = AddSpeakerToTalkForm(request.POST)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think in this case we might want to update the form name (or created an alias for this form with some other name that better suits this view)

{% endblock %}

{% block morejs %}
<script src="{% static "/js/taggit_labels_customised_with_max_number_of_tags.js" %}"></script>
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not needed for speakers, same for the CSS above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for catching this, removing!

@@ -32,7 +35,6 @@ def test_if_cfp_pages_are_login_required(client, url):
assert redirects_to(response, "/accounts/login/")


@mark.django_db
Copy link
Contributor

Choose a reason for hiding this comment

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

👍
what we could also do is make user_client fixture explicitly depend on db fixture which should do the same in a slightly more explicit way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would that mean that all django_db tests would have to accept user_client as an argument?

Copy link
Contributor

Choose a reason for hiding this comment

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

other way around – user_client would take db as argument, which it needs anyway because it creates users.

@umgelurgel umgelurgel force-pushed the issue-932-add-speaker-edit-view branch from c496867 to 681d708 Compare May 7, 2019 18:48
Copy link
Contributor

@artcz artcz left a comment

Choose a reason for hiding this comment

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

LGTM

@artcz artcz merged commit 01609cc into dev/ep2019 May 8, 2019
@umgelurgel umgelurgel deleted the issue-932-add-speaker-edit-view branch May 15, 2019 09:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Request: Edit the name of the speaker while updating the CFP
2 participants