Fix navigation between all users and journal users#4671
Conversation
|
@Astronaut-resting Is it alright if I assign you for first review of this? It fixes the link from the journal level to the press level, and also adds a box at the press level with links back to the journal users pages. |
|
Ah, I had a thought about this one: we may need to ensure the list of journal links is sorted A-Z. |
| {% comment %} The above URLs are hardcoded because there is no other | ||
| way to form a press URL dynamically on a journal page at the moment, | ||
| or to form a journal URL dynamically on a press page.{% endcomment %} |
There was a problem hiding this comment.
we do have one: see fqdn.external_journal_url.
| @override_settings(URL_CONFIG='domain') | ||
| def test_link_from_journal_users_to_all_users_domain(self): | ||
| self.client.force_login(self.press_manager) | ||
| self.journal_one.domain = 'one.example.org' | ||
| self.journal_one.save() | ||
| response = self.client.get( | ||
| '/user/all/', | ||
| SERVER_NAME=self.journal_one.domain, | ||
| ) | ||
| content = response.content.decode() | ||
| self.assertIn(f'{self.press.domain}/user/all/', content) | ||
|
|
||
| @override_settings(URL_CONFIG='path') | ||
| def test_link_from_journal_users_to_all_users_path(self): | ||
| self.client.force_login(self.press_manager) | ||
| response = self.client.get( | ||
| '/user/all/', | ||
| SERVER_NAME=self.journal_one.domain, | ||
| ) | ||
| content = response.content.decode() | ||
| self.assertIn(f'{self.press.domain}/user/all/', content) | ||
|
|
||
| @override_settings(URL_CONFIG='domain') | ||
| def test_link_from_all_users_to_journal_users_domain(self): | ||
| self.client.force_login(self.press_manager) | ||
| self.journal_one.domain = 'one.example.org' | ||
| self.journal_one.save() | ||
| response = self.client.get( | ||
| '/user/all/', | ||
| SERVER_NAME=self.press.domain, | ||
| ) | ||
| content = response.content.decode() | ||
| self.assertIn(f'one.example.org/user/all/', content) | ||
|
|
||
| @override_settings(URL_CONFIG='path') | ||
| def test_link_from_all_users_to_journal_users_path(self): | ||
| self.client.force_login(self.press_manager) | ||
| response = self.client.get( | ||
| '/user/all/', | ||
| SERVER_NAME=self.press.domain, | ||
| ) | ||
| content = response.content.decode() | ||
| self.assertIn( | ||
| f'{self.press.domain}/{self.journal_one.code}/user/all/', | ||
| content | ||
| ) |
There was a problem hiding this comment.
While I appreciate the existance of tests, these types of tests we are introducing here look more like acceptance tests than unit tests. They are relatively slow and they are just testing that you've put the correct URL on the template, so I don't think we should include them as long as we use the correct template tag to generate urls and that the tag itself is properly unittested.
|
Thanks for your updates Mauro. Before you review this Andy, I will fix a typo and also sort the journals A-Z. |
joemull
force-pushed
the
b-4667-press-users-link-bugfix
branch
from
e6dc7cf to
8f40daf
Compare
ajrbyers
left a comment
ajrbyers
left a comment
There was a problem hiding this comment.
Looks good - though added a comment about an potentially unused method.
4 participants
Fixes #4667 - bug report.
Fixes #4577 - underlying problem.
Adds an additional menu for links back from the press to the journal level:
"Test A" in this screenshot is the name of the press.