Repository: mwclient/mwclient

Issue:
Update Page state on save()
I find the following counter-intuitive:

```
> page = site.pages['Some new page']
> page.save('Some text')
> page.exists
False
> page.text()
''
> page.touched()
 (0, 0, 0, 0, 0, 0, 0, 0)
```

and so on.. After a save, the page does not reflect the updated state of the page. This could be solved by making extra API call to update page properties after save. For users trying to minimize the number of API calls, there should possibly be an option to disable this update..

Discussion:
@msj2: I too am finding same Error.

page.text() gives the text from Wiki page
I update the text & call page.save with 2 params (text, 'summary text')
page.save won't save
'On wikipedia, history won't even show the edit.

page.save returns.. 

{u'nochange': u'', u'contentmodel': u'wikitext', u'pageid': XXXX, u'result': u'Success', u'title':....)...
}

nochange ..implies changes weren't made??? 

I call page.touched.
It returns non-zero time,.. 
Touch Status: 
time.struct_time(tm_year=2017, tm_mon=9, tm_mday=7, tm_hour=6, tm_min=9, tm_sec=12, tm_wday=3, tm_yday=250, tm_isdst=-1)

Help me see, what I'm doing wrong.@danmichaelo: Your problem is a different one. The API response says that you send back the original text, not the modified text. Without more code I cannot say anything more.@azurstarshine: Essentially, this is a cache invalidation problem. A save should invalidate the cache, so the simplest fix would be to just *discard* all the state on `save`. Then when the caller invokes a function to obtain state, it forces the object (cache) to update. This would also avoid the overhead of making unnecessary requests, since they would only be made when the user attempts to access the data. It would not incur more overhead than fully updating the object, either, since the requests would have been made anyway.

Solution patch:
diff --git a/mwclient/listing.py b/mwclient/listing.py
index ef5780f..546d8c4 100644
--- a/mwclient/listing.py
+++ b/mwclient/listing.py
@@ -1,5 +1,5 @@
 from typing import (  # noqa: F401
-    Optional, Tuple, Any, Union, Iterator, Mapping, Iterable, Type
+    Optional, Tuple, Any, Union, Iterator, Mapping, Iterable, Type, Dict
 )
 
 import mwclient.image
@@ -237,7 +237,7 @@ class Category(mwclient.page.Page, GeneratorList):
         namespace: Optional[Namespace] = None
     ) -> None:
         mwclient.page.Page.__init__(self, site, name, info)
-        kwargs = {}
+        kwargs = {}  # type: Dict[str, Any]
         kwargs['gcmtitle'] = self.name
         if namespace:
             kwargs['gcmnamespace'] = namespace
diff --git a/mwclient/page.py b/mwclient/page.py
index edac3e9..4dfee70 100644
--- a/mwclient/page.py
+++ b/mwclient/page.py
@@ -1,6 +1,6 @@
 import time
 from typing import (  # noqa: F401
-    Optional, Mapping, Any, cast, Dict, Union, Tuple, Iterable, List
+    Optional, Mapping, Any, cast, Dict, Union, Tuple, Iterable, List, NoReturn
 )
 
 import mwclient.errors
@@ -73,15 +73,7 @@ class Page:
             raise mwclient.errors.InvalidPageTitle(info.get('invalidreason'))
 
         self.namespace = info.get('ns', 0)
-        self.name = info.get('title', '')
-        if self.namespace:
-            self.page_title = self.strip_namespace(self.name)
-        else:
-            self.page_title = self.name
-
-        self.base_title = self.page_title.split('/')[0]
-        self.base_name = self.name.split('/')[0]
-
+        self.name = info.get('title', '')  # type: str
         self.touched = parse_timestamp(info.get('touched'))
         self.revision = info.get('lastrevid', 0)
         self.exists = 'missing' not in info
@@ -100,6 +92,21 @@ class Page:
         self.last_rev_time = None  # type: Optional[time.struct_time]
         self.edit_time = None  # type: Optional[time.struct_time]
 
+    @property
+    def page_title(self) -> str:
+        if self.namespace:
+            return self.strip_namespace(self.name)
+        else:
+            return self.name
+
+    @property
+    def base_title(self) -> str:
+        return self.page_title.split('/')[0]
+
+    @property
+    def base_name(self) -> str:
+        return self.name.split('/')[0]
+
     def redirects_to(self) -> Optional['Page']:
         """ Get the redirect target page, or None if the page is not a redirect."""
         info = self.site.get('query', prop='pageprops', titles=self.name, redirects='')
@@ -286,7 +293,7 @@ class Page:
         if self.site.force_login:
             data['assert'] = 'user'
 
-        def do_edit() -> Any:
+        def do_edit() -> Dict[str, Any]:
             result = self.site.post('edit', title=self.name, summary=summary,
                                     token=self.get_token('edit'),
                                     **data)
@@ -307,9 +314,16 @@ class Page:
             else:
                 self.handle_edit_error(e, summary)
 
+        self.exists = True
+        self.name = result['edit'].get('title', self.name)
+        self.pageid = result['edit'].get('pageid', self.pageid)
+        self.revision = result['edit'].get('newrevid', self.revision)
+        self.contentmodel = result['edit'].get('contentmodel', self.contentmodel)
         # 'newtimestamp' is not included if no change was made
         if 'newtimestamp' in result['edit'].keys():
-            self.last_rev_time = parse_timestamp(result['edit'].get('newtimestamp'))
+            new_timestamp = parse_timestamp(result['edit'].get('newtimestamp'))
+            self.last_rev_time = new_timestamp
+            self.touched = new_timestamp
 
         # Workaround for https://phabricator.wikimedia.org/T211233
         for cookie in self.site.connection.cookies:
@@ -321,7 +335,7 @@ class Page:
         self._textcache = {}
         return result['edit']
 
-    def handle_edit_error(self, e: 'mwclient.errors.APIError', summary: str) -> None:
+    def handle_edit_error(self, e: 'mwclient.errors.APIError', summary: str) -> NoReturn:
         if e.code == 'editconflict':
             raise mwclient.errors.EditError(self, summary, e.info)
         elif e.code in {'protectedtitle', 'cantcreate', 'cantcreate-anon',
@@ -376,8 +390,15 @@ class Page:
             data['movesubpages'] = '1'
         if ignore_warnings:
             data['ignorewarnings'] = '1'
+
         result = self.site.post('move', ('from', self.name), to=new_title,
                                 token=self.get_token('move'), reason=reason, **data)
+
+        if 'redirectcreated' in result['move']:
+            self.redirect = True
+        else:
+            self.exists = False
+
         return result['move']
 
     def delete(
@@ -404,9 +425,12 @@ class Page:
             data['unwatch'] = '1'
         if oldimage:
             data['oldimage'] = oldimage
+
         result = self.site.post('delete', title=self.name,
                                 token=self.get_token('delete'),
                                 reason=reason, **data)
+
+        self.exists = False
         return result['delete']
 
     def purge(self) -> None:


Test patch:
diff --git a/test/test_page.py b/test/test_page.py
index 69149ae..a6ad61a 100644
--- a/test/test_page.py
+++ b/test/test_page.py
@@ -1,6 +1,9 @@
+import time
 import unittest
 import unittest.mock as mock
+
 import pytest
+
 import mwclient
 from mwclient.errors import APIError, AssertUserFailedError, ProtectedPageError, \
     InvalidPageTitle
@@ -211,6 +214,123 @@ class TestPage(unittest.TestCase):
         with pytest.raises(mwclient.errors.EditError):
             page.edit('Some text')
 
+    @mock.patch('mwclient.client.Site')
+    def test_edit(self, mock_site):
+        mock_site.blocked = False
+        mock_site.rights = ['read', 'edit']
+        mock_site.get.return_value = {'query': {'pages': {
+            '-1': {'ns': 1, 'title': 'Talk:Some page/Archive 1', 'missing': ''}
+        }}}
+        page = Page(mock_site, 'Talk:Some page/Archive 1')
+
+        mock_site.post.return_value = {
+            'edit': {'result': 'Success', 'pageid': 1234,
+                     'title': 'Talk:Some page/Archive 1', 'contentmodel': 'wikitext',
+                     'oldrevid': 123456, 'newrevid': 123457,
+                     'newtimestamp': '2024-10-02T12:34:07Z'}
+
+        }
+        page.edit('Some text')
+
+        mock_site.post.assert_called_once()
+        assert page.exists, 'Page should exist after edit'
+        assert page.pageid == 1234
+        assert page.name == 'Talk:Some page/Archive 1'
+        assert page.page_title == 'Some page/Archive 1'
+        assert page.base_title == 'Some page'
+        assert page.base_name == 'Talk:Some page'
+        assert page.contentmodel == 'wikitext'
+        assert page.revision == 123457
+        assert page.last_rev_time == time.struct_time(
+            (2024, 10, 2, 12, 34, 7, 2, 276, -1)
+        )
+        assert page.touched == time.struct_time(
+            (2024, 10, 2, 12, 34, 7, 2, 276, -1)
+        )
+
+    @mock.patch('mwclient.client.Site')
+    def test_delete(self, mock_site):
+        mock_site.rights = ['read', 'delete']
+        page_title = 'Some page'
+        page = Page(mock_site, page_title, info={
+            'contentmodel': 'wikitext',
+            'counter': '',
+            'lastrevid': 13355471,
+            'length': 58487,
+            'ns': 0,
+            'pageid': 728,
+            'pagelanguage': 'nb',
+            'protection': [],
+            'title': page_title,
+            'touched': '2014-09-14T21:11:52Z'
+        })
+
+        reason = 'Some reason'
+        mock_site.post.return_value = {
+            'delete': {'title': page_title, 'reason': reason, 'logid': 1234}
+        }
+        page.delete(reason)
+
+        mock_site.post.assert_called_once_with(
+            'delete', title=page_title, reason=reason, token=mock.ANY
+        )
+        assert not page.exists, 'Page should not exist after delete'
+
+    @mock.patch('mwclient.client.Site')
+    def test_move(self, mock_site):
+        mock_site.rights = ['read', 'move']
+        page_title = 'Some page'
+        page = Page(mock_site, page_title, info={
+            'contentmodel': 'wikitext',
+            'counter': '',
+            'lastrevid': 13355471,
+            'length': 58487,
+            'ns': 0,
+            'pageid': 728,
+            'pagelanguage': 'nb',
+            'protection': [],
+            'title': page_title,
+            'touched': '2014-09-14T21:11:52Z'
+        })
+
+        new_title = 'Some new page'
+        reason = 'Some reason'
+        mock_site.post.return_value = {
+            'move': {'from': page_title, 'to': new_title, 'reason': reason,
+                     'redirectcreated': ''}
+        }
+        page.move(new_title, reason)
+
+        assert page.exists, 'Page should still exist after move'
+        assert page.redirect, 'Page should be a redirect after move'
+
+    @mock.patch('mwclient.client.Site')
+    def test_move_no_redirect(self, mock_site):
+        mock_site.rights = ['read', 'move']
+        page_title = 'Some page'
+        page = Page(mock_site, page_title, info={
+            'contentmodel': 'wikitext',
+            'counter': '',
+            'lastrevid': 13355471,
+            'length': 58487,
+            'ns': 0,
+            'pageid': 728,
+            'pagelanguage': 'nb',
+            'protection': [],
+            'title': page_title,
+            'touched': '2014-09-14T21:11:52Z'
+        })
+
+        new_title = 'Some new page'
+        reason = 'Some reason'
+        mock_site.post.return_value = {
+            'move': {'from': page_title, 'to': new_title, 'reason': reason}
+        }
+        page.move(new_title, reason, no_redirect=True)
+
+        assert not page.exists, 'Page should not exist after move'
+        assert not page.redirect, 'Page should not be a redirect after move'
+
 
 class TestPageApiArgs(unittest.TestCase):